Re: [PATCH] dax: constify the struct device_type usage
On 2/19/24 5:47 AM, Ricardo B. Marliere wrote:
> Since commit aed65af1cc2f ("drivers: make device_type const"), the driver
> core can properly handle constant struct device_type. Move the
> dax_mapping_type variable to be a constant structure as well, placing it
> into read-only memory which can not be modified at runtime.
>
> Cc: Greg Kroah-Hartman
> Signed-off-by: Ricardo B. Marliere
Reviewed-by: Dave Jiang
> ---
> drivers/dax/bus.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> index 1ff1ab5fa105..e265ba019785 100644
> --- a/drivers/dax/bus.c
> +++ b/drivers/dax/bus.c
> @@ -763,7 +763,7 @@ static const struct attribute_group
> *dax_mapping_attribute_groups[] = {
> NULL,
> };
>
> -static struct device_type dax_mapping_type = {
> +static const struct device_type dax_mapping_type = {
> .release = dax_mapping_release,
> .groups = dax_mapping_attribute_groups,
> };
>
> ---
> base-commit: b401b621758e46812da61fa58a67c3fd8d91de0d
> change-id: 20240219-device_cleanup-dax-d82fd0c67ffd
>
> Best regards,
Re: [PATCHv2 1/2] dt-bindings: usb: typec: anx7688: start a binding document
On Fri, 23 Feb 2024 22:28:42 +0100, Pavel Machek wrote: > Add binding for anx7688 usb type-c bridge. I don't have a datasheet, > but I did best I could. > > Signed-off-by: Pavel Machek > > --- > > v2: implement review feedback > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/usb/analogix,anx7688.example.dtb: typec@2c: 'hdmi_vt-supply' does not match any of the regexes: 'pinctrl-[0-9]+' from schema $id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/ZdkOCqPKqa/u9...@duo.ucw.cz The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
[PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge
From: Ondrej Jirman This is driver for ANX7688 USB-C HDMI, with flashing and debugging features removed. ANX7688 is rather criticial piece on PinePhone, there's no display and no battery charging without it. There's likely more work to be done here, but having basic support in mainline is needed to be able to work on the other stuff (networking, cameras, power management). Signed-off-by: Ondrej Jirman Co-developed-by: Martijn Braam Co-developed-by: Samuel Holland Signed-off-by: Pavel Machek --- v2: Fix checkpatch stuff. Some cleanups, adapt to dts format in 1/2. diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig index 2f80c2792dbd..c9043ae61546 100644 --- a/drivers/usb/typec/Kconfig +++ b/drivers/usb/typec/Kconfig @@ -64,6 +64,17 @@ config TYPEC_ANX7411 If you choose to build this driver as a dynamically linked module, the module will be called anx7411.ko. +config TYPEC_ANX7688 + tristate "Analogix ANX7688 Type-C DRP Port controller and mux driver" + depends on I2C + depends on USB_ROLE_SWITCH + help + Say Y or M here if your system has Analogix ANX7688 Type-C Bridge + controller driver. + + If you choose to build this driver as a dynamically linked module, the + module will be called anx7688.ko. + config TYPEC_RT1719 tristate "Richtek RT1719 Sink Only Type-C controller driver" depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile index 7a368fea61bc..3f8ff94ad294 100644 --- a/drivers/usb/typec/Makefile +++ b/drivers/usb/typec/Makefile @@ -7,6 +7,7 @@ obj-$(CONFIG_TYPEC_TCPM)+= tcpm/ obj-$(CONFIG_TYPEC_UCSI) += ucsi/ obj-$(CONFIG_TYPEC_TPS6598X) += tipd/ obj-$(CONFIG_TYPEC_ANX7411)+= anx7411.o +obj-$(CONFIG_TYPEC_ANX7688)+= anx7688.o obj-$(CONFIG_TYPEC_HD3SS3220) += hd3ss3220.o obj-$(CONFIG_TYPEC_STUSB160X) += stusb160x.o obj-$(CONFIG_TYPEC_RT1719) += rt1719.o diff --git a/drivers/usb/typec/anx7688.c b/drivers/usb/typec/anx7688.c new file mode 100644 index ..14d033bbc0c7 --- /dev/null +++ b/drivers/usb/typec/anx7688.c @@ -0,0 +1,1927 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * ANX7688 USB-C HDMI bridge/PD driver + * + * Warning, this driver is somewhat PinePhone specific. + * + * How this works: + * - this driver allows to program firmware into ANX7688 EEPROM, and + * initialize it + * - it then communicates with the firmware running on the OCM (on-chip + * microcontroller) + * - it detects whether there is cable plugged in or not and powers + * up or down the ANX7688 based on that + * - when the cable is connected the firmware on the OCM will handle + * the detection of the nature of the device on the other end + * of the USB-C cable + * - this driver then communicates with the USB phy to let it swap + * data roles accordingly + * - it also enables VBUS and VCONN regulators as appropriate + * - USB phy driver (Allwinner) needs to know whether to switch to + * device or host mode, or whether to turn off + * - when the firmware detects SRC.1.5A or SRC.3.0A via CC pins + * or something else via PD, it notifies this driver via software + * interrupt and this driver will determine how to update the TypeC + * port status and what input current limit is appropriate + * - input current limit determination happens 500ms after cable + * insertion or hard reset (delay is necessary to determine whether + * the remote end is PD capable or not) + * - this driver tells to the PMIC driver that the input current limit + * needs to be changed + * - this driver also monitors PMIC status and re-sets the input current + * limit if it changes for some reason (due to PMIC internal decision + * making) (this is disabled for now) + * + * ANX7688 FW behavior as observed: + * + * - DO NOT SET MORE THAN 1 SINK CAPABILITY! Firmware will ignore what + * you set and send hardcoded PDO_BATT 5-21V 30W message! + * + * Product brief: + * https://www.analogix.com/en/system/files/AA-002281-PB-6-ANX7688_Product_Brief_0.pdf + * Development notes: + * https://xnux.eu/devices/feature/anx7688.html + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/* firmware regs */ + +#define ANX7688_REG_VBUS_OFF_DELAY_TIME 0x22 +#define ANX7688_REG_FEATURE_CTRL0x27 +#define ANX7688_REG_EEPROM_LOAD_STATUS1 0x11 +#define ANX7688_REG_EEPROM_LOAD_STATUS0 0x12 +#define ANX7688_REG_FW_VERSION1 0x15 +#define ANX7688_REG_FW_VERSION0 0x16 + +#define ANX7688_EEPROM_FW_LOADED 0x01 + +#define ANX7688_REG_STATUS_INT_MASK 0x17 +#define ANX7688_REG_STATUS_INT 0x28 +#define ANX7688_IRQS_RECEIVED_MSG BIT(0) +#define ANX7688_IRQS_RECEIVED_ACK BIT(1) +#define ANX7688_IRQS_VCONN_CHANGE BIT(2) +#define
[PATCHv2 1/2] dt-bindings: usb: typec: anx7688: start a binding document
Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
but I did best I could.
Signed-off-by: Pavel Machek
---
v2: implement review feedback
diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
new file mode 100644
index ..9e887eafb5fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
@@ -0,0 +1,127 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+# Pin names can be deduced from
+#
https://files.pine64.org/doc/PinePhone/PinePhone%20v1.2b%20Released%20Schematic.pdf
+
+title: Analogix ANX7688 Type-C controller
+
+maintainers:
+ - Pavel Machek
+
+properties:
+ compatible:
+enum:
+ - analogix,anx7688
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+ reset-gpios:
+maxItems: 1
+description: GPIO controlling RESET_N (B7) pin.
+
+ enable-gpios:
+maxItems: 1
+description: GPIO controlling POWER_EN (D2) pin.
+
+ cabledet-gpios:
+maxItems: 1
+description: GPIO controlling CABLE_DET (C3) pin.
+
+ avdd10-supply:
+description: 1.0V power supply going to AVDD10 (A4, ...) pins
+
+ dvdd10-supply:
+description: 1.0V power supply going to DVDD10 (D6, ...) pins
+
+ avdd18-supply:
+description: 1.8V power supply going to AVDD18 (E3, ...) pins
+
+ dvdd18-supply:
+description: 1.8V power supply going to DVDD18 (G4, ...) pins
+
+ avdd33-supply:
+description: 3.3V power supply going to AVDD33 (C4, ...) pins
+
+ i2c-supply: true
+ vconn-supply: true
+ hdmi-vt-supply: true
+ vbus-supply: true
+ vbus-in-supply: true
+
+ connector:
+type: object
+$ref: /schemas/connector/usb-connector.yaml
+
+description:
+ Properties for usb c connector.
+
+properties:
+ compatible:
+const: usb-c-connector
+
+required:
+ - compatible
+ - reg
+ - connector
+
+additionalProperties: false
+
+examples:
+ - |
+#include
+#include
+
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+typec@2c {
+compatible = "analogix,anx7688";
+reg = <0x2c>;
+interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+interrupt-parent = <>;
+
+enable-gpios = < 3 10 GPIO_ACTIVE_LOW>; /* PD10 */
+reset-gpios = < 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
+cabledet-gpios = <_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
+
+avdd10-supply = <_anx1v0>;
+dvdd10-supply = <_anx1v0>;
+avdd18-supply = <_ldo_io1>;
+dvdd18-supply = <_ldo_io1>;
+avdd33-supply = <_dcdc1>;
+i2c-supply = <_ldo_io0>;
+vconn-supply = <_vconn5v0>;
+hdmi_vt-supply = <_dldo1>;
+
+vbus-supply = <_usb_5v>;
+vbus-in-supply = <_power_supply>;
+
+typec_con: connector {
+compatible = "usb-c-connector";
+power-role = "dual";
+data-role = "dual";
+try-power-role = "source";
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+port@0 {
+reg = <0>;
+typec_con_ep: endpoint {
+remote-endpoint = <_hs_ep>;
+};
+};
+};
+};
+};
+};
+...
--
People of Russia, stop Putin before his war on Ukraine escalates.
signature.asc
Description: PGP signature
[PATCH] tracing: Remove second parameter to __assign_rel_str()
From: "Steven Rostedt (Google)"
The second parameter of __assign_rel_str() is no longer used. It can be removed.
Note, the only real users of rel_string is user events. This code is just
in the sample code for testing purposes.
This makes __assign_rel_str() different than __assign_str() but that's
fine. __assign_str() is used over 700 places and has a larger impact. That
change will come later.
Signed-off-by: Steven Rostedt (Google)
---
include/trace/stages/stage6_event_callback.h | 2 +-
samples/trace_events/trace-events-sample.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/trace/stages/stage6_event_callback.h
b/include/trace/stages/stage6_event_callback.h
index 935608971899..a0c15f67eabe 100644
--- a/include/trace/stages/stage6_event_callback.h
+++ b/include/trace/stages/stage6_event_callback.h
@@ -91,7 +91,7 @@
#define __rel_string_len(item, src, len) __rel_dynamic_array(char, item, -1)
#undef __assign_rel_str
-#define __assign_rel_str(dst, src) \
+#define __assign_rel_str(dst) \
do {\
char *__str__ = __get_rel_str(dst); \
int __len__ = __get_rel_dynamic_array_len(dst) - 1; \
diff --git a/samples/trace_events/trace-events-sample.h
b/samples/trace_events/trace-events-sample.h
index 2dfaf7fc7bfa..500981eca74d 100644
--- a/samples/trace_events/trace-events-sample.h
+++ b/samples/trace_events/trace-events-sample.h
@@ -574,7 +574,7 @@ TRACE_EVENT(foo_rel_loc,
),
TP_fast_assign(
- __assign_rel_str(foo, foo);
+ __assign_rel_str(foo);
__entry->bar = bar;
__assign_rel_bitmask(bitmask, mask,
BITS_PER_BYTE * sizeof(unsigned long));
--
2.43.0
[PATCH v2] tracing: Add warning if string in __assign_str() does not match __string()
From: "Steven Rostedt (Google)"
In preparation to remove the second parameter of __assign_str(), make sure
it is really a duplicate of __string() by adding a WARN_ON_ONCE().
Signed-off-by: Steven Rostedt (Google)
---
Changes since v1:
https://lore.kernel.org/linux-trace-kernel/20240223154821.77b24...@gandalf.local.home/
- If src is NULL it is saved as NULL, no need to not test that too.
include/trace/stages/stage6_event_callback.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/trace/stages/stage6_event_callback.h
b/include/trace/stages/stage6_event_callback.h
index 0c0f50bcdc56..935608971899 100644
--- a/include/trace/stages/stage6_event_callback.h
+++ b/include/trace/stages/stage6_event_callback.h
@@ -35,6 +35,7 @@
do {\
char *__str__ = __get_str(dst); \
int __len__ = __get_dynamic_array_len(dst) - 1; \
+ WARN_ON_ONCE((src) != __data_offsets.dst##_ptr_); \
memcpy(__str__, __data_offsets.dst##_ptr_ ? : \
EVENT_NULL_STR, __len__);\
__str__[__len__] = '\0';\
--
2.43.0
[PATCH] tracing: Add warning if string in __assign_str() does not match __string()
From: "Steven Rostedt (Google)"
In preparation to remove the second parameter of __assign_str(), make sure
it is really a duplicate of __string() by adding a WARN_ON_ONCE().
Signed-off-by: Steven Rostedt (Google)
---
include/trace/stages/stage6_event_callback.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/trace/stages/stage6_event_callback.h
b/include/trace/stages/stage6_event_callback.h
index 0c0f50bcdc56..7372e2c2a0c4 100644
--- a/include/trace/stages/stage6_event_callback.h
+++ b/include/trace/stages/stage6_event_callback.h
@@ -35,6 +35,7 @@
do {\
char *__str__ = __get_str(dst); \
int __len__ = __get_dynamic_array_len(dst) - 1; \
+ WARN_ON_ONCE((src) && (src) != __data_offsets.dst##_ptr_); \
memcpy(__str__, __data_offsets.dst##_ptr_ ? : \
EVENT_NULL_STR, __len__);\
__str__[__len__] = '\0';\
--
2.43.0
Re: [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On Fri, 23 Feb 2024 13:46:53 -0500
Steven Rostedt wrote:
> Now one thing I could do is to not remove the parameter, but just add:
>
> WARN_ON_ONCE((src) != __data_offsets->item##_ptr_);
>
> in the __assign_str() macro to make sure that it's still the same that is
> assigned. But I'm not sure how useful that is, and still causes burden to
> have it. I never really liked the passing of the string in two places to
> begin with.
Hmm, maybe I'll just add this patch for 6.9 and then in 6.10 do the
parameter removal.
-- Steve
diff --git a/include/trace/stages/stage6_event_callback.h
b/include/trace/stages/stage6_event_callback.h
index 0c0f50bcdc56..7372e2c2a0c4 100644
--- a/include/trace/stages/stage6_event_callback.h
+++ b/include/trace/stages/stage6_event_callback.h
@@ -35,6 +35,7 @@ #define __assign_str(dst, src)
do {\
char *__str__ = __get_str(dst); \
int __len__ = __get_dynamic_array_len(dst) - 1; \
+ WARN_ON_ONCE((src) != __data_offsets.dst##_ptr_); \
memcpy(__str__, __data_offsets.dst##_ptr_ ? : \
EVENT_NULL_STR, __len__);\
__str__[__len__] = '\0';\
[PATCH] tracing: Add __string_len() example
From: "Steven Rostedt (Google)"
There's no example code that uses __string_len(), and since the sample
code is used for testing the event logic, add a use case.
Signed-off-by: Steven Rostedt (Google)
---
samples/trace_events/trace-events-sample.h | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/samples/trace_events/trace-events-sample.h
b/samples/trace_events/trace-events-sample.h
index f2d2d56ce8e2..2dfaf7fc7bfa 100644
--- a/samples/trace_events/trace-events-sample.h
+++ b/samples/trace_events/trace-events-sample.h
@@ -303,6 +303,7 @@ TRACE_EVENT(foo_bar,
__bitmask( cpus, num_possible_cpus() )
__cpumask( cpum)
__vstring( vstr, fmt,va )
+ __string_len( lstr, foo,bar / 2 < strlen(foo) ? bar / 2
: strlen(foo) )
),
TP_fast_assign(
@@ -311,12 +312,13 @@ TRACE_EVENT(foo_bar,
memcpy(__get_dynamic_array(list), lst,
__length_of(lst) * sizeof(int));
__assign_str(str, string);
+ __assign_str(lstr, foo);
__assign_vstr(vstr, fmt, va);
__assign_bitmask(cpus, cpumask_bits(mask), num_possible_cpus());
__assign_cpumask(cpum, cpumask_bits(mask));
),
- TP_printk("foo %s %d %s %s %s %s (%s) (%s) %s", __entry->foo,
__entry->bar,
+ TP_printk("foo %s %d %s %s %s %s %s (%s) (%s) %s", __entry->foo,
__entry->bar,
/*
* Notice here the use of some helper functions. This includes:
@@ -360,7 +362,8 @@ TRACE_EVENT(foo_bar,
__print_array(__get_dynamic_array(list),
__get_dynamic_array_len(list) / sizeof(int),
sizeof(int)),
- __get_str(str), __get_bitmask(cpus), __get_cpumask(cpum),
+ __get_str(str), __get_str(lstr),
+ __get_bitmask(cpus), __get_cpumask(cpum),
__get_str(vstr))
);
--
2.43.0
[PATCH v2] tracing: Remove __assign_str_len()
From: "Steven Rostedt (Google)"
Now that __assign_str() gets the length from the __string() (and
__string_len()) macros, there's no reason to have a separate
__assign_str_len() macro as __assign_str() can get the length of the
string needed.
Also remove __assign_rel_str() although it had no users anyway.
Signed-off-by: Steven Rostedt (Google)
---
Changes since v2:
https://lore.kernel.org/linux-trace-kernel/20240223144840.56366...@gandalf.local.home/
- I just realized that __assign_str_len() makes sure that the string ends
with a '\0'. To use __assign_str(), it too must write a '\0' at the end
of the string.
fs/nfsd/trace.h | 8 +++---
include/trace/stages/stage6_event_callback.h | 28
samples/trace_events/trace-events-sample.h | 9 ---
3 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 2cd57033791f..98b14f30d772 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -102,7 +102,7 @@ TRACE_EVENT(nfsd_compound,
TP_fast_assign(
__entry->xid = be32_to_cpu(rqst->rq_xid);
__entry->opcnt = opcnt;
- __assign_str_len(tag, tag, taglen);
+ __assign_str(tag, tag);
),
TP_printk("xid=0x%08x opcnt=%u tag=%s",
__entry->xid, __entry->opcnt, __get_str(tag)
@@ -483,7 +483,7 @@ TRACE_EVENT(nfsd_dirent,
TP_fast_assign(
__entry->fh_hash = fhp ? knfsd_fh_hash(>fh_handle) : 0;
__entry->ino = ino;
- __assign_str_len(name, name, namlen)
+ __assign_str(name, name);
),
TP_printk("fh_hash=0x%08x ino=%llu name=%s",
__entry->fh_hash, __entry->ino, __get_str(name)
@@ -853,7 +853,7 @@ DECLARE_EVENT_CLASS(nfsd_clid_class,
__entry->flavor = clp->cl_cred.cr_flavor;
memcpy(__entry->verifier, (void *)>cl_verifier,
NFS4_VERIFIER_SIZE);
- __assign_str_len(name, clp->cl_name.data, clp->cl_name.len);
+ __assign_str(name, clp->cl_name.data);
),
TP_printk("addr=%pISpc name='%s' verifier=0x%s flavor=%s
client=%08x:%08x",
__entry->addr, __get_str(name),
@@ -1800,7 +1800,7 @@ TRACE_EVENT(nfsd_ctl_time,
TP_fast_assign(
__entry->netns_ino = net->ns.inum;
__entry->time = time;
- __assign_str_len(name, name, namelen);
+ __assign_str(name, name);
),
TP_printk("file=%s time=%d\n",
__get_str(name), __entry->time
diff --git a/include/trace/stages/stage6_event_callback.h
b/include/trace/stages/stage6_event_callback.h
index 2bfd49713b42..0c0f50bcdc56 100644
--- a/include/trace/stages/stage6_event_callback.h
+++ b/include/trace/stages/stage6_event_callback.h
@@ -32,16 +32,13 @@
#undef __assign_str
#define __assign_str(dst, src) \
- memcpy(__get_str(dst), __data_offsets.dst##_ptr_ ? : EVENT_NULL_STR, \
- __get_dynamic_array_len(dst))
-
-#undef __assign_str_len
-#define __assign_str_len(dst, src, len)
\
do {\
- memcpy(__get_str(dst), \
- __data_offsets.dst##_ptr_ ? : EVENT_NULL_STR, len); \
- __get_str(dst)[len] = '\0'; \
- } while(0)
+ char *__str__ = __get_str(dst); \
+ int __len__ = __get_dynamic_array_len(dst) - 1; \
+ memcpy(__str__, __data_offsets.dst##_ptr_ ? : \
+ EVENT_NULL_STR, __len__);\
+ __str__[__len__] = '\0';\
+ } while (0)
#undef __assign_vstr
#define __assign_vstr(dst, fmt, va)\
@@ -94,15 +91,12 @@
#undef __assign_rel_str
#define __assign_rel_str(dst, src) \
- memcpy(__get_rel_str(dst), __data_offsets.dst##_ptr_ ? :
EVENT_NULL_STR, \
- __get_rel_dynamic_array_len(dst))
-
-#undef __assign_rel_str_len
-#define __assign_rel_str_len(dst, src, len)\
do {\
- memcpy(__get_rel_str(dst), \
- __data_offsets.dst##_ptr_ ? : EVENT_NULL_STR, len); \
- __get_rel_str(dst)[len] = '\0'; \
+ char *__str__ = __get_rel_str(dst); \
+ int __len__ = __get_rel_dynamic_array_len(dst) - 1; \
+ memcpy(__str__, __data_offsets.dst##_ptr_ ? : \
+ EVENT_NULL_STR, __len__);\
+
Re: [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On Fri, 23 Feb 2024 14:50:49 -0500
Kent Overstreet wrote:
> Tangentially related though, what would make me really happy is if we
> could create the string with in the TP__fast_assign() section. I have to
> have a bunch of annoying wrappers right now because the string length
> has to be known when we invoke the tracepoint.
You can use __string_len() to determine the string length in the tracepoint
(which is executed in the TP_fast_assign() section).
My clean up patches will make __assign_str_len() obsolete too (I'm working
on them now), and you can just use __assign_str().
I noticed that I don't have a string_len example in the sample code and I'm
actually writing it now.
// cutting out everything else:
TRACE_EVENT(foo_bar,
TP_PROTO(const char *foo, int bar),
TP_ARGS(foo, bar),
TP_STRUCT__entry(
__string_len( lstr, foo,bar < strlen(foo) ? bar :
strlen(foo) )
),
TP_fast_assign(
__assign_str(lstr, foo);
// Note, the above is with my updates, without them, you need to duplicate the
logic
// __assign_str_len(lstr, foo, bar < strlen(foo) ? bar :
strlen(foo));
),
TP_printk("%s", __get_str(lstr))
);
The above will allocate "bar < strlen(foo) ? bar : strlen(foo)" size on the
ring buffer. As the size is already stored, my clean up code uses that
instead of requiring duplicating the logic again.
-- Steve
Re: [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On Fri, Feb 23, 2024 at 01:46:53PM -0500, Steven Rostedt wrote: > On Fri, 23 Feb 2024 10:30:45 -0800 > Jeff Johnson wrote: > > > On 2/23/2024 9:56 AM, Steven Rostedt wrote: > > > From: "Steven Rostedt (Google)" > > > > > > [ > > >This is a treewide change. I will likely re-create this patch again in > > >the second week of the merge window of v6.9 and submit it then. Hoping > > >to keep the conflicts that it will cause to a minimum. > > > ] > > > > > > With the rework of how the __string() handles dynamic strings where it > > > saves off the source string in field in the helper structure[1], the > > > assignment of that value to the trace event field is stored in the helper > > > value and does not need to be passed in again. > > > > Just curious if this could be done piecemeal by first changing the > > macros to be variadic macros which allows you to ignore the extra > > argument. The callers could then be modified in their separate trees. > > And then once all the callers have be merged, the macros could be > > changed to no longer be variadic. > > I weighed doing that, but I think ripping off the band-aid is a better > approach. One thing I found is that leaving unused parameters in the macros > can cause bugs itself. I found one case doing my clean up, where an unused > parameter in one of the macros was bogus, and when I made it a used > parameter, it broke the build. > > I think for tree-wide changes, the preferred approach is to do one big > patch at once. And since this only affects TRACE_EVENT() macros, it > hopefully would not be too much of a burden (although out of tree users may > suffer from this, but do we care?) Agreed on doing it all at once, it'll be way less spam for people to deal with. Tangentially related though, what would make me really happy is if we could create the string with in the TP__fast_assign() section. I have to have a bunch of annoying wrappers right now because the string length has to be known when we invoke the tracepoint.
[PATCH] tracing: Remove __assign_str_len()
From: "Steven Rostedt (Google)"
Now that __assign_str() gets the length from the __string() (and
__string_len()) macros, there's no reason to have a separate
__assign_str_len() macro as __assign_str() can get the length of the
string needed.
Signed-off-by: Steven Rostedt (Google)
---
fs/nfsd/trace.h | 8
include/trace/stages/stage6_event_callback.h | 16
samples/trace_events/trace-events-sample.h | 9 +
3 files changed, 9 insertions(+), 24 deletions(-)
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 2cd57033791f..98b14f30d772 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -102,7 +102,7 @@ TRACE_EVENT(nfsd_compound,
TP_fast_assign(
__entry->xid = be32_to_cpu(rqst->rq_xid);
__entry->opcnt = opcnt;
- __assign_str_len(tag, tag, taglen);
+ __assign_str(tag, tag);
),
TP_printk("xid=0x%08x opcnt=%u tag=%s",
__entry->xid, __entry->opcnt, __get_str(tag)
@@ -483,7 +483,7 @@ TRACE_EVENT(nfsd_dirent,
TP_fast_assign(
__entry->fh_hash = fhp ? knfsd_fh_hash(>fh_handle) : 0;
__entry->ino = ino;
- __assign_str_len(name, name, namlen)
+ __assign_str(name, name);
),
TP_printk("fh_hash=0x%08x ino=%llu name=%s",
__entry->fh_hash, __entry->ino, __get_str(name)
@@ -853,7 +853,7 @@ DECLARE_EVENT_CLASS(nfsd_clid_class,
__entry->flavor = clp->cl_cred.cr_flavor;
memcpy(__entry->verifier, (void *)>cl_verifier,
NFS4_VERIFIER_SIZE);
- __assign_str_len(name, clp->cl_name.data, clp->cl_name.len);
+ __assign_str(name, clp->cl_name.data);
),
TP_printk("addr=%pISpc name='%s' verifier=0x%s flavor=%s
client=%08x:%08x",
__entry->addr, __get_str(name),
@@ -1800,7 +1800,7 @@ TRACE_EVENT(nfsd_ctl_time,
TP_fast_assign(
__entry->netns_ino = net->ns.inum;
__entry->time = time;
- __assign_str_len(name, name, namelen);
+ __assign_str(name, name);
),
TP_printk("file=%s time=%d\n",
__get_str(name), __entry->time
diff --git a/include/trace/stages/stage6_event_callback.h
b/include/trace/stages/stage6_event_callback.h
index 2bfd49713b42..61e718962036 100644
--- a/include/trace/stages/stage6_event_callback.h
+++ b/include/trace/stages/stage6_event_callback.h
@@ -35,14 +35,6 @@
memcpy(__get_str(dst), __data_offsets.dst##_ptr_ ? : EVENT_NULL_STR, \
__get_dynamic_array_len(dst))
-#undef __assign_str_len
-#define __assign_str_len(dst, src, len)
\
- do {\
- memcpy(__get_str(dst), \
- __data_offsets.dst##_ptr_ ? : EVENT_NULL_STR, len); \
- __get_str(dst)[len] = '\0'; \
- } while(0)
-
#undef __assign_vstr
#define __assign_vstr(dst, fmt, va)\
do {\
@@ -97,14 +89,6 @@
memcpy(__get_rel_str(dst), __data_offsets.dst##_ptr_ ? :
EVENT_NULL_STR, \
__get_rel_dynamic_array_len(dst))
-#undef __assign_rel_str_len
-#define __assign_rel_str_len(dst, src, len)\
- do {\
- memcpy(__get_rel_str(dst), \
- __data_offsets.dst##_ptr_ ? : EVENT_NULL_STR, len); \
- __get_rel_str(dst)[len] = '\0'; \
- } while (0)
-
#undef __rel_bitmask
#define __rel_bitmask(item, nr_bits) __rel_dynamic_array(unsigned long, item,
-1)
diff --git a/samples/trace_events/trace-events-sample.h
b/samples/trace_events/trace-events-sample.h
index 23f923ccd529..f2d2d56ce8e2 100644
--- a/samples/trace_events/trace-events-sample.h
+++ b/samples/trace_events/trace-events-sample.h
@@ -163,8 +163,7 @@
* __string().
*
* __string_len: This is a helper to a __dynamic_array, but it understands
- *that the array has characters in it, and with the combined
- * use of __assign_str_len(), it will allocate 'len' + 1 bytes
+ *that the array has characters in it, it will allocate 'len' + 1 bytes
* in the ring buffer and add a '\0' to the string. This is
* useful if the string being saved has no terminating '\0' byte.
* It requires that the length of the string is known as it acts
@@ -174,9 +173,11 @@
*
* __string_len(foo, bar, len)
*
- * To assign this string, use the helper macro __assign_str_len().
+ * To assign this string, use the helper macro __assign_str().
Re: [PATCH v9 13/15] x86/sgx: Turn on per-cgroup EPC reclamation
On Thu, 22 Feb 2024 16:44:45 -0600, Huang, Kai wrote:
On 23/02/2024 5:36 am, Haitao Huang wrote:
On Wed, 21 Feb 2024 05:23:00 -0600, Huang, Kai
wrote:
On Mon, 2024-02-05 at 13:06 -0800, Haitao Huang wrote:
From: Kristen Carlson Accardi
Previous patches have implemented all infrastructure needed for
per-cgroup EPC page tracking and reclaiming. But all reclaimable EPC
pages are still tracked in the global LRU as sgx_lru_list() returns
hard
coded reference to the global LRU.
Change sgx_lru_list() to return the LRU of the cgroup in which the
given
EPC page is allocated.
This makes all EPC pages tracked in per-cgroup LRUs and the global
reclaimer (ksgxd) will not be able to reclaim any pages from the
global
LRU. However, in cases of over-committing, i.e., sum of cgroup limits
greater than the total capacity, cgroups may never reclaim but the
total
usage can still be near the capacity. Therefore global reclamation is
still needed in those cases and it should reclaim from the root
cgroup.
Modify sgx_reclaim_pages_global(), to reclaim from the root EPC cgroup
when cgroup is enabled, otherwise from the global LRU.
Similarly, modify sgx_can_reclaim(), to check emptiness of LRUs of all
cgroups when EPC cgroup is enabled, otherwise only check the global
LRU.
With these changes, the global reclamation and per-cgroup reclamation
both work properly with all pages tracked in per-cgroup LRUs.
Co-developed-by: Sean Christopherson
Signed-off-by: Sean Christopherson
Signed-off-by: Kristen Carlson Accardi
Co-developed-by: Haitao Huang
Signed-off-by: Haitao Huang
---
V7:
- Split this out from the big patch, #10 in V6. (Dave, Kai)
---
arch/x86/kernel/cpu/sgx/main.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/sgx/main.c
b/arch/x86/kernel/cpu/sgx/main.c
index 6b0c26cac621..d4265a390ba9 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -34,12 +34,23 @@ static struct sgx_epc_lru_list sgx_global_lru;
static inline struct sgx_epc_lru_list *sgx_lru_list(struct
sgx_epc_page *epc_page)
{
+#ifdef CONFIG_CGROUP_SGX_EPC
+if (epc_page->epc_cg)
+return _page->epc_cg->lru;
+
+/* This should not happen if kernel is configured correctly */
+WARN_ON_ONCE(1);
+#endif
return _global_lru;
}
How about when EPC cgroup is enabled, but one enclave doesn't belong
to any EPC
cgroup? Is it OK to track EPC pages for these enclaves to the root
EPC cgroup's
LRU list together with other enclaves belongs to the root cgroup?
This should be a valid case, right?
There is no such case. Each page is in the root by default.
Is it guaranteed by the (misc) cgroup design/implementation? If so
please add this information to the changelog and/or comments? It helps
non-cgroup expert like me to understand.
Will do
Thanks
Haitao
Re: [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On Fri, 23 Feb 2024 10:30:45 -0800 Jeff Johnson wrote: > On 2/23/2024 9:56 AM, Steven Rostedt wrote: > > From: "Steven Rostedt (Google)" > > > > [ > >This is a treewide change. I will likely re-create this patch again in > >the second week of the merge window of v6.9 and submit it then. Hoping > >to keep the conflicts that it will cause to a minimum. > > ] > > > > With the rework of how the __string() handles dynamic strings where it > > saves off the source string in field in the helper structure[1], the > > assignment of that value to the trace event field is stored in the helper > > value and does not need to be passed in again. > > Just curious if this could be done piecemeal by first changing the > macros to be variadic macros which allows you to ignore the extra > argument. The callers could then be modified in their separate trees. > And then once all the callers have be merged, the macros could be > changed to no longer be variadic. I weighed doing that, but I think ripping off the band-aid is a better approach. One thing I found is that leaving unused parameters in the macros can cause bugs itself. I found one case doing my clean up, where an unused parameter in one of the macros was bogus, and when I made it a used parameter, it broke the build. I think for tree-wide changes, the preferred approach is to do one big patch at once. And since this only affects TRACE_EVENT() macros, it hopefully would not be too much of a burden (although out of tree users may suffer from this, but do we care?) Now one thing I could do is to not remove the parameter, but just add: WARN_ON_ONCE((src) != __data_offsets->item##_ptr_); in the __assign_str() macro to make sure that it's still the same that is assigned. But I'm not sure how useful that is, and still causes burden to have it. I never really liked the passing of the string in two places to begin with. -- Steve
Re: [PATCH v3 7/7] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
On Fri, Feb 23, 2024 at 02:54:13PM +0100, Arnaud POULIQUEN wrote:
> Hello Mathieu,
>
> On 2/22/24 20:02, Mathieu Poirier wrote:
> > Hi,
> >
> > On Wed, Feb 14, 2024 at 06:21:27PM +0100, Arnaud Pouliquen wrote:
> >> The new TEE remoteproc device is used to manage remote firmware in a
> >> secure, trusted context. The 'st,stm32mp1-m4-tee' compatibility is
> >> introduced to delegate the loading of the firmware to the trusted
> >> execution context. In such cases, the firmware should be signed and
> >> adhere to the image format defined by the TEE.
> >>
> >> A new "to_attach" field is introduced to differentiate the use cases
> >> "firmware loaded by the boot stage" and "firmware loaded by the TEE".
> >>
> >> Signed-off-by: Arnaud Pouliquen
> >> ---
> >> V2 to V3 update:
> >> - remove stm32_rproc_tee_elf_sanity_check(), stm32_rproc_tee_elf_load()
> >> stm32_rproc_tee_elf_find_loaded_rsc_table() and stm32_rproc_tee_start()
> >> that are bnow unused
> >> - use new rproc::alt_boot field to sepcify that the alternate fboot method
> >> is used
> >> - use stm32_rproc::to_attach field to differenciate attch mode from
> >> remoteproc tee boot mode.
> >> - remove the used of stm32_rproc::fw_loaded
> >> ---
> >> drivers/remoteproc/stm32_rproc.c | 85 +---
> >> 1 file changed, 79 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/remoteproc/stm32_rproc.c
> >> b/drivers/remoteproc/stm32_rproc.c
> >> index fcc0001e2657..9cfcf66462e0 100644
> >> --- a/drivers/remoteproc/stm32_rproc.c
> >> +++ b/drivers/remoteproc/stm32_rproc.c
> >> @@ -20,6 +20,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >> #include
> >>
> >> #include "remoteproc_internal.h"
> >> @@ -49,6 +50,9 @@
> >> #define M4_STATE_STANDBY 4
> >> #define M4_STATE_CRASH5
> >>
> >> +/* Remote processor unique identifier aligned with the Trusted Execution
> >> Environment definitions */
> >> +#define STM32_MP1_M4_PROC_ID0
> >> +
> >> struct stm32_syscon {
> >>struct regmap *map;
> >>u32 reg;
> >> @@ -90,6 +94,8 @@ struct stm32_rproc {
> >>struct stm32_mbox mb[MBOX_NB_MBX];
> >>struct workqueue_struct *workqueue;
> >>bool hold_boot_smc;
> >> + bool to_attach;
> >> + struct tee_rproc *trproc;
> >>void __iomem *rsc_va;
> >> };
> >>
> >> @@ -253,10 +259,30 @@ static int stm32_rproc_release(struct rproc *rproc)
> >>return err;
> >>}
> >>}
> >> + ddata->to_attach = false;
> >>
> >>return err;
> >> }
> >>
> >> +static int stm32_rproc_tee_attach(struct rproc *rproc)
> >> +{
> >> + /* Nothing to do, remote proc already started by the secured context. */
> >> + return 0;
> >> +}
> >> +
> >> +static int stm32_rproc_tee_stop(struct rproc *rproc)
> >> +{
> >> + int err;
> >> +
> >> + stm32_rproc_request_shutdown(rproc);
> >> +
> >> + err = tee_rproc_stop(rproc);
> >> + if (err)
> >> + return err;
> >> +
> >> + return stm32_rproc_release(rproc);
> >> +}
> >> +
> >> static int stm32_rproc_prepare(struct rproc *rproc)
> >> {
> >>struct device *dev = rproc->dev.parent;
> >> @@ -637,10 +663,14 @@ stm32_rproc_get_loaded_rsc_table(struct rproc
> >> *rproc, size_t *table_sz)
> >> {
> >>struct stm32_rproc *ddata = rproc->priv;
> >>struct device *dev = rproc->dev.parent;
> >> + struct tee_rproc *trproc = ddata->trproc;
> >>phys_addr_t rsc_pa;
> >>u32 rsc_da;
> >>int err;
> >>
> >> + if (trproc && !ddata->to_attach)
> >> + return tee_rproc_get_loaded_rsc_table(rproc, table_sz);
> >> +
> >
> > Why do we need a flag at all? Why can't
> > st_rproc_tee_ops::get_loaded_rsc_table
> > be set to tee_rproc_get_loaded_rsc_table()?
>
>
> This function is used to retrieve the address of the resource table in 3 cases
> - attach to a firmware started by the boot loader (U-boot).
> - load of the firmware by OP-TEE.
> - crash recovery on a signed firmware started by the boot loader.
>
> The flag is used to differentiate the attch from the other uses cases
> For instance we support this use case.
> 1) attach to the firmware on boot
> 2) crash during runtime
> 2a) stop the firmware by OP-TEE( ddata->to_attach set to 0)
> 2b) load the firmware by OP-TEE
> 2c) get the loaded resource table from OP-TEE (we can not guaranty
> that the firmware loaded on recovery is the same)
> 2d) restart the firmware by OP-TEE
This is not maintainable and needs to be broken down into smaller
building blocks. The introduction of tee_rproc_parse_fw() should help dealing
with some of the complexity.
>
> >
> >>/* The resource table has already been mapped, nothing to do */
> >>if (ddata->rsc_va)
> >>goto done;
> >> @@ -693,8 +723,20 @@ static const struct rproc_ops st_rproc_ops = {
> >>.get_boot_addr = rproc_elf_get_boot_addr,
> >> };
> >>
> >> +static const struct rproc_ops st_rproc_tee_ops = {
> >> + .prepare= stm32_rproc_prepare,
> >> +
Re: [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On 2/23/2024 9:56 AM, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > [ >This is a treewide change. I will likely re-create this patch again in >the second week of the merge window of v6.9 and submit it then. Hoping >to keep the conflicts that it will cause to a minimum. > ] > > With the rework of how the __string() handles dynamic strings where it > saves off the source string in field in the helper structure[1], the > assignment of that value to the trace event field is stored in the helper > value and does not need to be passed in again. Just curious if this could be done piecemeal by first changing the macros to be variadic macros which allows you to ignore the extra argument. The callers could then be modified in their separate trees. And then once all the callers have be merged, the macros could be changed to no longer be variadic.
Re: [PATCH v3 3/7] remoteproc: core: Add check on cached_table pointer
On Wed, Feb 14, 2024 at 06:21:23PM +0100, Arnaud Pouliquen wrote:
> Add a check on the optional rproc->cached_table to perform the memory
> copy only if it is not null.
>
> 2 use cases to support:
> - starting on boot, in which case rproc->cached_table can be null,
> - starting on crash recovery, where the cached table is used to save
> the resource table configuration on stop and re-apply the configuration
> on the re-start.
>
> Signed-off-by: Arnaud Pouliquen
> ---
> drivers/remoteproc/remoteproc_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/remoteproc/remoteproc_core.c
> b/drivers/remoteproc/remoteproc_core.c
> index 283ca071e35c..34b0093689da 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1278,7 +1278,7 @@ static int rproc_start(struct rproc *rproc, const
> struct firmware *fw)
>* that any subsequent changes will be applied to the loaded version.
>*/
> loaded_table = rproc_find_loaded_rsc_table(rproc, fw);
> - if (loaded_table) {
> + if (loaded_table && rproc->cached_table) {
... and this becomes:
if (loaded_table != rproc->cached_table)
with a detailed comment about what is going on and a reference to
tee_rproc_parse_fw().
There are other things to adjust in this patchset but starting with that will
hopefully deal with a few of them. We can address the rest at the next
iteration.
I am done reviewing this set.
Thanks,
Mathieu
> memcpy(loaded_table, rproc->cached_table, rproc->table_sz);
> rproc->table_ptr = loaded_table;
> }
> --
> 2.25.1
>
Re: [PATCH v3 1/7] remoteproc: Add TEE support
On Wed, Feb 14, 2024 at 06:21:21PM +0100, Arnaud Pouliquen wrote: > From: Arnaud Pouliquen > > Add a remoteproc TEE (Trusted Execution Environment) driver > that will be probed by the TEE bus. If the associated Trusted > application is supported on secure part this device offers a client > interface to load a firmware in the secure part. > This firmware could be authenticated and decrypted by the secure > trusted application. > > Signed-off-by: Arnaud Pouliquen > --- > update from V2 > - Use 'tee_rproc' prefix for all functions > - rename tee_rproc_get_loaded_rsc_table to tee_rproc_find_loaded_rsc_table > - redefine fonction to better match with the rproc_ops structure format > - replace 'struct tee_rproc' parameter by 'struct rproc' parameter > - rename 'rproc_tee_get_rsc_table()' to tee_rproc_get_loaded_rsc_table() > and rework it to remove the cached_table management. > - introduce tee_rproc_get_context() to get the tee_rproc struct from the > rproc struct > - rename tee_rproc_get_loaded_rsc_table() to > tee_rproc_find_loaded_rsc_table() > - remove useless check on tee_rproc_ctx structure in tee_rproc_register() > and tee_rproc_unregister() > - fix test on the return of tee_rproc_ctx = devm_kzalloc() > - remove useless includes and unused tee_rproc_mem structure. > --- > drivers/remoteproc/Kconfig | 9 + > drivers/remoteproc/Makefile | 1 + > drivers/remoteproc/tee_remoteproc.c | 397 > include/linux/tee_remoteproc.h | 102 +++ > 4 files changed, 509 insertions(+) > create mode 100644 drivers/remoteproc/tee_remoteproc.c > create mode 100644 include/linux/tee_remoteproc.h > > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig > index 48845dc8fa85..85299606806c 100644 > --- a/drivers/remoteproc/Kconfig > +++ b/drivers/remoteproc/Kconfig > @@ -365,6 +365,15 @@ config XLNX_R5_REMOTEPROC > > It's safe to say N if not interested in using RPU r5f cores. > > + > +config TEE_REMOTEPROC > + tristate "trusted firmware support by a TEE application" > + depends on OPTEE > + help > + Support for trusted remote processors firmware. The firmware > + authentication and/or decryption are managed by a trusted application. > + This can be either built-in or a loadable module. > + > endif # REMOTEPROC > > endmenu > diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile > index 91314a9b43ce..fa8daebce277 100644 > --- a/drivers/remoteproc/Makefile > +++ b/drivers/remoteproc/Makefile > @@ -36,6 +36,7 @@ obj-$(CONFIG_RCAR_REMOTEPROC) += rcar_rproc.o > obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o > obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o > obj-$(CONFIG_STM32_RPROC)+= stm32_rproc.o > +obj-$(CONFIG_TEE_REMOTEPROC) += tee_remoteproc.o > obj-$(CONFIG_TI_K3_DSP_REMOTEPROC) += ti_k3_dsp_remoteproc.o > obj-$(CONFIG_TI_K3_R5_REMOTEPROC)+= ti_k3_r5_remoteproc.o > obj-$(CONFIG_XLNX_R5_REMOTEPROC) += xlnx_r5_remoteproc.o > diff --git a/drivers/remoteproc/tee_remoteproc.c > b/drivers/remoteproc/tee_remoteproc.c > new file mode 100644 > index ..ac727e062d00 > --- /dev/null > +++ b/drivers/remoteproc/tee_remoteproc.c > @@ -0,0 +1,397 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) STMicroelectronics 2023 - All Rights Reserved > + * Author: Arnaud Pouliquen > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "remoteproc_internal.h" > + > +#define MAX_TEE_PARAM_ARRY_MEMBER4 > + > +/* > + * Authentication of the firmware and load in the remote processor memory > + * > + * [in] params[0].value.a: unique 32bit identifier of the remote processor > + * [in] params[1].memref: buffer containing the image of the > buffer > + */ > +#define TA_RPROC_FW_CMD_LOAD_FW 1 > + > +/* > + * Start the remote processor > + * > + * [in] params[0].value.a: unique 32bit identifier of the remote processor > + */ > +#define TA_RPROC_FW_CMD_START_FW 2 > + > +/* > + * Stop the remote processor > + * > + * [in] params[0].value.a: unique 32bit identifier of the remote processor > + */ > +#define TA_RPROC_FW_CMD_STOP_FW 3 > + > +/* > + * Return the address of the resource table, or 0 if not found > + * No check is done to verify that the address returned is accessible by > + * the non secure context. If the resource table is loaded in a protected > + * memory the access by the non secure context will lead to a data abort. > + * > + * [in] params[0].value.a: unique 32bit identifier of the remote processor > + * [out] params[1].value.a: 32bit LSB resource table memory address > + * [out] params[1].value.b: 32bit MSB resource table memory address > + * [out] params[2].value.a: 32bit LSB resource table memory size > + * [out] params[2].value.b: 32bit
Re: [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On Fri, 23 Feb 2024 12:56:34 -0500 Steven Rostedt wrote: > Note, the same updates will need to be done for: > > __assign_str_len() > __assign_rel_str() > __assign_rel_str_len() Correction: The below macros do not pass in their source to the entry macros, so they will not need to be updated. -- Steve > __assign_bitmask() > __assign_rel_bitmask() > __assign_cpumask() > __assign_rel_cpumask()
Re: [PATCH v2] mm: Update mark_victim tracepoints fields
On Thu, Feb 22, 2024 at 9:59 AM Carlos Galo wrote:
>
> On Thu, Feb 22, 2024 at 6:16 AM Michal Hocko wrote:
> >
> > On Wed 21-02-24 13:30:51, Carlos Galo wrote:
> > > On Tue, Feb 20, 2024 at 11:55 PM Michal Hocko wrote:
> > > >
> > > > Hi,
> > > > sorry I have missed this before.
> > > >
> > > > On Thu 11-01-24 21:05:30, Carlos Galo wrote:
> > > > > The current implementation of the mark_victim tracepoint provides only
> > > > > the process ID (pid) of the victim process. This limitation poses
> > > > > challenges for userspace tools that need additional information
> > > > > about the OOM victim. The association between pid and the additional
> > > > > data may be lost after the kill, making it difficult for userspace to
> > > > > correlate the OOM event with the specific process.
> > > >
> > > > You are correct that post OOM all per-process information is lost. On
> > > > the other hand we do dump all this information to the kernel log. Could
> > > > you explain why that is not suitable for your purpose?
> > >
> > > Userspace tools often need real-time visibility into OOM situations
> > > for userspace intervention. Our use case involves utilizing BPF
> > > programs, along with BPF ring buffers, to provide OOM notification to
> > > userspace. Parsing kernel logs would be significant overhead as
> > > opposed to the event based BPF approach.
> >
> > Please put that into the changelog.
>
> Will do.
>
> > > > > In order to mitigate this limitation, add the following fields:
> > > > >
> > > > > - UID
> > > > >In Android each installed application has a unique UID. Including
> > > > >the `uid` assists in correlating OOM events with specific apps.
> > > > >
> > > > > - Process Name (comm)
> > > > >Enables identification of the affected process.
> > > > >
> > > > > - OOM Score
> > > > >Allows userspace to get additional insights of the relative kill
> > > > >priority of the OOM victim.
> > > >
> > > > What is the oom score useful for?
> > > >
> > > The OOM score provides us a measure of the victim's importance. On the
> > > android side, it allows us to identify if top or foreground apps are
> > > killed, which have user perceptible impact.
> >
> > But the value on its own (wihtout knowing scores of other tasks) doesn't
> > really tell you anything, does it?
>
> Android uses the OOM adj_score ranges to categorize app state
> (foreground, background, ...). I'll resend a v3 with the commit text
> updated to include details about this.
>
> > > > Is there any reason to provide a different information from the one
> > > > reported to the kernel log?
> > > > __oom_kill_process:
> > > > pr_err("%s: Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB,
> > > > file-rss:%lukB, shmem-rss:%lukB, UID:%u pgtables:%lukB
> > > > oom_score_adj:%hd\n",
> > > > message, task_pid_nr(victim), victim->comm,
> > > > K(mm->total_vm),
> > > > K(get_mm_counter(mm, MM_ANONPAGES)),
> > > > K(get_mm_counter(mm, MM_FILEPAGES)),
> > > > K(get_mm_counter(mm, MM_SHMEMPAGES)),
> > > > from_kuid(_user_ns, task_uid(victim)),
> > > > mm_pgtables_bytes(mm) >> 10,
> > > > victim->signal->oom_score_adj);
> > > >
> > >
> > > We added these fields we need (UID, process name, and OOM score), but
> > > we're open to adding the others if you prefer that for consistency
> > > with the kernel log.
> >
> > yes, I think the consistency would be better here. For one it reports
> > numbers which can tell quite a lot about the killed victim. It is a
> > superset of what you already asking for. With a notable exception of the
> > oom_score which is really dubious without a wider context.
>
> Sounds good, I'll resend a v3 that includes these fields as well.
>
> Thanks,
> Carlos
>
I posted V3 here:
https://lore.kernel.org/all/20240223173258.174828-1-carlosg...@google.com/
Thanks,
Carlos
> > --
> > Michal Hocko
> > SUSE Labs
[PATCH v3] mm: Update mark_victim tracepoints fields
The current implementation of the mark_victim tracepoint provides only
the process ID (pid) of the victim process. This limitation poses
challenges for userspace tools requiring real-time OOM analysis and
intervention. Although this information is available from the kernel
logs, it’s not the appropriate format to provide OOM notifications. In
Android, BPF programs are used with the mark_victim trace events to
notify userspace of an OOM kill. For consistency, update the trace
event to include the same information about the OOMed victim as the
kernel logs.
- UID
In Android each installed application has a unique UID. Including
the `uid` assists in correlating OOM events with specific apps.
- Process Name (comm)
Enables identification of the affected process.
- OOM Score
Will allow userspace to get additional insight of the relative kill
priority of the OOM victim. In Android, the oom_score_adj is used to
categorize app state (foreground, background, etc.), which aids in
analyzing user-perceptible impacts of OOM events [1].
- Total VM, RSS Stats, and pgtables
Amount of memory used by the victim that will, potentially, be freed up
by killing it.
[1]
https://cs.android.com/android/platform/superproject/main/+/246dc8fc95b6d93afcba5c6d6c133307abb3ac2e:frameworks/base/services/core/java/com/android/server/am/ProcessList.java;l=188-283
Cc: Steven Rostedt
Cc: Andrew Morton
Cc: Suren Baghdasaryan
Cc: Michal Hocko
Reviewed-by: Steven Rostedt
Signed-off-by: Carlos Galo
---
v3:
- Added total_vm, rss fields, and pgtables per Michal Hocko.
- Added Steven Rostedt reviewed by
- Updated commit description to include android usecase
v2: Fixed build error. Added missing comma when printing `__entry->uid`.
include/trace/events/oom.h | 36
mm/oom_kill.c | 6 +-
2 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/include/trace/events/oom.h b/include/trace/events/oom.h
index 26a11e4a2c36..b799f3bcba82 100644
--- a/include/trace/events/oom.h
+++ b/include/trace/events/oom.h
@@ -7,6 +7,8 @@
#include
#include
+#define PG_COUNT_TO_KB(x) ((x) << (PAGE_SHIFT - 10))
+
TRACE_EVENT(oom_score_adj_update,
TP_PROTO(struct task_struct *task),
@@ -72,19 +74,45 @@ TRACE_EVENT(reclaim_retry_zone,
);
TRACE_EVENT(mark_victim,
- TP_PROTO(int pid),
+ TP_PROTO(struct task_struct *task, uid_t uid),
- TP_ARGS(pid),
+ TP_ARGS(task, uid),
TP_STRUCT__entry(
__field(int, pid)
+ __string(comm, task->comm)
+ __field(unsigned long, total_vm)
+ __field(unsigned long, anon_rss)
+ __field(unsigned long, file_rss)
+ __field(unsigned long, shmem_rss)
+ __field(uid_t, uid)
+ __field(unsigned long, pgtables)
+ __field(short, oom_score_adj)
),
TP_fast_assign(
- __entry->pid = pid;
+ __entry->pid = task->pid;
+ __assign_str(comm, task->comm);
+ __entry->total_vm = PG_COUNT_TO_KB(task->mm->total_vm);
+ __entry->anon_rss = PG_COUNT_TO_KB(get_mm_counter(task->mm,
MM_ANONPAGES));
+ __entry->file_rss = PG_COUNT_TO_KB(get_mm_counter(task->mm,
MM_FILEPAGES));
+ __entry->shmem_rss = PG_COUNT_TO_KB(get_mm_counter(task->mm,
MM_SHMEMPAGES));
+ __entry->uid = uid;
+ __entry->pgtables = mm_pgtables_bytes(task->mm) >> 10;
+ __entry->oom_score_adj = task->signal->oom_score_adj;
),
- TP_printk("pid=%d", __entry->pid)
+ TP_printk("pid=%d comm=%s total-vm=%lukB anon-rss=%lukB file-rss:%lukB
shmem-rss:%lukB uid=%u pgtables=%lukB oom_score_adj=%hd",
+ __entry->pid,
+ __get_str(comm),
+ __entry->total_vm,
+ __entry->anon_rss,
+ __entry->file_rss,
+ __entry->shmem_rss,
+ __entry->uid,
+ __entry->pgtables,
+ __entry->oom_score_adj
+ )
);
TRACE_EVENT(wake_reaper,
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 91ccd82097c2..8d6a207c3c59 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -44,6 +44,7 @@
#include
#include
#include
+#include
#include
#include "internal.h"
@@ -754,6 +755,7 @@ static inline void queue_oom_reaper(struct task_struct *tsk)
*/
static void mark_oom_victim(struct task_struct *tsk)
{
+ const struct cred *cred;
struct mm_struct *mm = tsk->mm;
WARN_ON(oom_killer_disabled);
@@ -773,7 +775,9 @@ static void mark_oom_victim(struct task_struct *tsk)
*/
__thaw_task(tsk);
atomic_inc(_victims);
- trace_mark_victim(tsk->pid);
+ cred = get_task_cred(tsk);
+ trace_mark_victim(tsk, cred->uid.val);
+ put_cred(cred);
}
/**
--
2.44.0.rc0.258.g7320e95886-goog
Re: [PATCH v9 10/15] x86/sgx: Add EPC reclamation in cgroup try_charge()
On Fri, 23 Feb 2024 04:18:18 -0600, Huang, Kai wrote: > Right. When code reaches to here, we already passed reclaim per cgroup. Yes if try_charge() failed we must do pre-cgroup reclaim. The cgroup may not at or reach limit but system has run out of physical EPC. But after try_charge() we can still choose to reclaim from the current group, but not necessarily have to be global, right? I am not sure whether I am missing something, but could you elaborate why we should choose to reclaim from the global? Once try_charge is done and returns zero that means the cgroup usage is charged and it's not over usage limit. So you really can't reclaim from that cgroup if allocation failed. The only thing you can do is to reclaim globally. This could happen when the sum of limits of all cgroups is greater than the physical EPC, i.e., user is overcommitting. Thanks Haitao
Re: [PATCH v3 0/7] Introduction of a remoteproc tee to load signed firmware
On 2/22/24 10:55, Naman Jain wrote: > On 2/22/2024 2:17 PM, Arnaud POULIQUEN wrote: >> Hello Naman, >> >> On 2/22/24 06:43, Naman Jain wrote: >>> On 2/14/2024 10:51 PM, Arnaud Pouliquen wrote: Updates from the previous version [1]: This version proposes another approach based on an alternate load and boot of the coprocessor. Therefore, the constraint introduced by tee_remoteproc is that the firmware has to be authenticated and loaded before the resource table can be obtained. The existing boot sequence is: > 1) Get the resource table and store it in a cache, calling rproc->ops->parse_fw(). 2) Parse the resource table and handle resources, calling rproc_handle_resources. 3) Load the firmware, calling rproc->ops->load(). 4) Start the firmware, calling rproc->ops->start(). => Steps 1 and 2 are executed in rproc_fw_boot(), while steps 3 and 4 are executed in rproc_start(). => the use of rproc->ops->load() ops is mandatory The boot sequence needed for TEE boot is: 1) Load the firmware. 2) Get the loaded resource, no cache. 3) Parse the resource table and handle resources. 4) Start the firmware. >>> >>> Hi, >>> What problem are we really addressing here by reordering load, parse of >>> FW resources? >> >> The feature introduced in TEE is the signature of the firmware images. That >> means that before getting the resource table, we need to first authenticate >> the >> firmware images. >> Authenticating a firmware image means that we have to copy the firmware into >> protected memory that cannot be corrupted by the non-secure and then verify >> the >> signature. >> The strategy implemented in OP-TEE is to load the firmware into destination >> memory and then authenticate it. >> This strategy avoids having a temporary copy of the whole images in a secure >> memory. >> This strategy imposes loading the firmware images before retrieving the >> resource >> table. >> >>> Basically, what are the limitations of the current design you are referring >>> to? >>> I understood that TEE is designed that way. >> >> The limitation of the current design is that we obtain the resource table >> before >> loading the firmware. Following the current design would impose constraints >> in >> TEE that are not straightforward. Step 1 (getting the resource table and >> storing >> it in a cache) would require having a copy of the resource table in TEE after >> authenticating the images. However, authenticating the firmware, as explained >> before, depends on the strategy implemented. In TEE implementation, we load >> the >> firmware to authenticate it in the destination memory. >> >> Regards, >> Arnaud > > > Hello Arnaud, > I think now I got your point. In TEE, you don't want to do anything(read > resource table) with FW images, until its loaded and authenticated. > Since current design was not allowing you to do it, you had to reorganize the > code so that this can be achieved. > > Generally speaking, in current design, if authentication fails for some > reason later, one can handle it, but it depends on the implementation of > parse_fw op if the damage is already done. > > Please correct me if this is wrong assumption. That's correct. Regards, Arnaud > Patch looks good to me. > > Regards, > Naman Jain
Re: [PATCH v3 7/7] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware
Hello Mathieu,
On 2/22/24 20:02, Mathieu Poirier wrote:
> Hi,
>
> On Wed, Feb 14, 2024 at 06:21:27PM +0100, Arnaud Pouliquen wrote:
>> The new TEE remoteproc device is used to manage remote firmware in a
>> secure, trusted context. The 'st,stm32mp1-m4-tee' compatibility is
>> introduced to delegate the loading of the firmware to the trusted
>> execution context. In such cases, the firmware should be signed and
>> adhere to the image format defined by the TEE.
>>
>> A new "to_attach" field is introduced to differentiate the use cases
>> "firmware loaded by the boot stage" and "firmware loaded by the TEE".
>>
>> Signed-off-by: Arnaud Pouliquen
>> ---
>> V2 to V3 update:
>> - remove stm32_rproc_tee_elf_sanity_check(), stm32_rproc_tee_elf_load()
>> stm32_rproc_tee_elf_find_loaded_rsc_table() and stm32_rproc_tee_start()
>> that are bnow unused
>> - use new rproc::alt_boot field to sepcify that the alternate fboot method
>> is used
>> - use stm32_rproc::to_attach field to differenciate attch mode from
>> remoteproc tee boot mode.
>> - remove the used of stm32_rproc::fw_loaded
>> ---
>> drivers/remoteproc/stm32_rproc.c | 85 +---
>> 1 file changed, 79 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/remoteproc/stm32_rproc.c
>> b/drivers/remoteproc/stm32_rproc.c
>> index fcc0001e2657..9cfcf66462e0 100644
>> --- a/drivers/remoteproc/stm32_rproc.c
>> +++ b/drivers/remoteproc/stm32_rproc.c
>> @@ -20,6 +20,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>
>> #include "remoteproc_internal.h"
>> @@ -49,6 +50,9 @@
>> #define M4_STATE_STANDBY4
>> #define M4_STATE_CRASH 5
>>
>> +/* Remote processor unique identifier aligned with the Trusted Execution
>> Environment definitions */
>> +#define STM32_MP1_M4_PROC_ID0
>> +
>> struct stm32_syscon {
>> struct regmap *map;
>> u32 reg;
>> @@ -90,6 +94,8 @@ struct stm32_rproc {
>> struct stm32_mbox mb[MBOX_NB_MBX];
>> struct workqueue_struct *workqueue;
>> bool hold_boot_smc;
>> +bool to_attach;
>> +struct tee_rproc *trproc;
>> void __iomem *rsc_va;
>> };
>>
>> @@ -253,10 +259,30 @@ static int stm32_rproc_release(struct rproc *rproc)
>> return err;
>> }
>> }
>> +ddata->to_attach = false;
>>
>> return err;
>> }
>>
>> +static int stm32_rproc_tee_attach(struct rproc *rproc)
>> +{
>> +/* Nothing to do, remote proc already started by the secured context. */
>> +return 0;
>> +}
>> +
>> +static int stm32_rproc_tee_stop(struct rproc *rproc)
>> +{
>> +int err;
>> +
>> +stm32_rproc_request_shutdown(rproc);
>> +
>> +err = tee_rproc_stop(rproc);
>> +if (err)
>> +return err;
>> +
>> +return stm32_rproc_release(rproc);
>> +}
>> +
>> static int stm32_rproc_prepare(struct rproc *rproc)
>> {
>> struct device *dev = rproc->dev.parent;
>> @@ -637,10 +663,14 @@ stm32_rproc_get_loaded_rsc_table(struct rproc *rproc,
>> size_t *table_sz)
>> {
>> struct stm32_rproc *ddata = rproc->priv;
>> struct device *dev = rproc->dev.parent;
>> +struct tee_rproc *trproc = ddata->trproc;
>> phys_addr_t rsc_pa;
>> u32 rsc_da;
>> int err;
>>
>> +if (trproc && !ddata->to_attach)
>> +return tee_rproc_get_loaded_rsc_table(rproc, table_sz);
>> +
>
> Why do we need a flag at all? Why can't
> st_rproc_tee_ops::get_loaded_rsc_table
> be set to tee_rproc_get_loaded_rsc_table()?
This function is used to retrieve the address of the resource table in 3 cases
- attach to a firmware started by the boot loader (U-boot).
- load of the firmware by OP-TEE.
- crash recovery on a signed firmware started by the boot loader.
The flag is used to differentiate the attch from the other uses cases
For instance we support this use case.
1) attach to the firmware on boot
2) crash during runtime
2a) stop the firmware by OP-TEE( ddata->to_attach set to 0)
2b) load the firmware by OP-TEE
2c) get the loaded resource table from OP-TEE (we can not guaranty
that the firmware loaded on recovery is the same)
2d) restart the firmware by OP-TEE
>
>> /* The resource table has already been mapped, nothing to do */
>> if (ddata->rsc_va)
>> goto done;
>> @@ -693,8 +723,20 @@ static const struct rproc_ops st_rproc_ops = {
>> .get_boot_addr = rproc_elf_get_boot_addr,
>> };
>>
>> +static const struct rproc_ops st_rproc_tee_ops = {
>> +.prepare= stm32_rproc_prepare,
>> +.start = tee_rproc_start,
>> +.stop = stm32_rproc_tee_stop,
>> +.attach = stm32_rproc_tee_attach,
>> +.kick = stm32_rproc_kick,
>> +.get_loaded_rsc_table = stm32_rproc_get_loaded_rsc_table,
>> +.find_loaded_rsc_table = tee_rproc_find_loaded_rsc_table,
>> +.load = tee_rproc_load_fw,
>> +};
>> +
>> static const struct of_device_id stm32_rproc_match[] = {
>> -{
Re: [PATCH net-next 1/2] net/vsockmon: Leverage core stats allocator
On Fri, Feb 23, 2024 at 12:58 PM Breno Leitao wrote:
>
> With commit 34d21de99cea9 ("net: Move {l,t,d}stats allocation to core and
> convert veth & vrf"), stats allocation could be done on net core
> instead of this driver.
>
> With this new approach, the driver doesn't have to bother with error
> handling (allocation failure checking, making sure free happens in the
> right spot, etc). This is core responsibility now.
>
> Remove the allocation in the vsockmon driver and leverage the network
> core allocation instead.
>
> Signed-off-by: Breno Leitao
Reviewed-by: Eric Dumazet
Re: [PATCH net-next 2/2] net/vsockmon: Do not set zeroed statistics
On Fri, Feb 23, 2024 at 12:58 PM Breno Leitao wrote: > > Do not set rtnl_link_stats64 fields to zero, since they are zeroed > before ops->ndo_get_stats64 is called in core dev_get_stats() function. > > Signed-off-by: Breno Leitao Reviewed-by: Eric Dumazet
[PATCH net-next 2/2] net/vsockmon: Do not set zeroed statistics
Do not set rtnl_link_stats64 fields to zero, since they are zeroed
before ops->ndo_get_stats64 is called in core dev_get_stats() function.
Signed-off-by: Breno Leitao
---
drivers/net/vsockmon.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/vsockmon.c b/drivers/net/vsockmon.c
index a0b4dca36baf..a1ba5169ed5d 100644
--- a/drivers/net/vsockmon.c
+++ b/drivers/net/vsockmon.c
@@ -46,9 +46,6 @@ static void
vsockmon_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
{
dev_lstats_read(dev, >rx_packets, >rx_bytes);
-
- stats->tx_packets = 0;
- stats->tx_bytes = 0;
}
static int vsockmon_is_valid_mtu(int new_mtu)
--
2.39.3
[PATCH net-next 1/2] net/vsockmon: Leverage core stats allocator
With commit 34d21de99cea9 ("net: Move {l,t,d}stats allocation to core and
convert veth & vrf"), stats allocation could be done on net core
instead of this driver.
With this new approach, the driver doesn't have to bother with error
handling (allocation failure checking, making sure free happens in the
right spot, etc). This is core responsibility now.
Remove the allocation in the vsockmon driver and leverage the network
core allocation instead.
Signed-off-by: Breno Leitao
---
drivers/net/vsockmon.c | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/net/vsockmon.c b/drivers/net/vsockmon.c
index b1bb1b04b664..a0b4dca36baf 100644
--- a/drivers/net/vsockmon.c
+++ b/drivers/net/vsockmon.c
@@ -13,19 +13,6 @@
#define DEFAULT_MTU (VIRTIO_VSOCK_MAX_PKT_BUF_SIZE + \
sizeof(struct af_vsockmon_hdr))
-static int vsockmon_dev_init(struct net_device *dev)
-{
- dev->lstats = netdev_alloc_pcpu_stats(struct pcpu_lstats);
- if (!dev->lstats)
- return -ENOMEM;
- return 0;
-}
-
-static void vsockmon_dev_uninit(struct net_device *dev)
-{
- free_percpu(dev->lstats);
-}
-
struct vsockmon {
struct vsock_tap vt;
};
@@ -79,8 +66,6 @@ static int vsockmon_change_mtu(struct net_device *dev, int
new_mtu)
}
static const struct net_device_ops vsockmon_ops = {
- .ndo_init = vsockmon_dev_init,
- .ndo_uninit = vsockmon_dev_uninit,
.ndo_open = vsockmon_open,
.ndo_stop = vsockmon_close,
.ndo_start_xmit = vsockmon_xmit,
@@ -112,6 +97,7 @@ static void vsockmon_setup(struct net_device *dev)
dev->flags = IFF_NOARP;
dev->mtu = DEFAULT_MTU;
+ dev->pcpu_stat_type = NETDEV_PCPU_STAT_LSTATS;
}
static struct rtnl_link_ops vsockmon_link_ops __read_mostly = {
--
2.39.3
Re: [PATCH 2/4] csky: apply page shift to PFN instead of VA in pfn_to_virt
On Wed, Jan 31, 2024 at 2:28 PM Yan Zhao wrote:
>
> Apply the page shift to PFN to get physical address for final VA.
> The macro __va should take physical address instead of PFN as input.
>
> Fixes: c1884e1e1164 ("csky: Make pfn accessors static inlines")
> Signed-off-by: Yan Zhao
> ---
> arch/csky/include/asm/page.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/csky/include/asm/page.h b/arch/csky/include/asm/page.h
> index 4a0502e324a6..2c4cc7825a7b 100644
> --- a/arch/csky/include/asm/page.h
> +++ b/arch/csky/include/asm/page.h
> @@ -84,7 +84,7 @@ static inline unsigned long virt_to_pfn(const void *kaddr)
>
> static inline void * pfn_to_virt(unsigned long pfn)
> {
> - return (void *)((unsigned long)__va(pfn) << PAGE_SHIFT);
> + return __va(pfn << PAGE_SHIFT);
> }
Reviewed-by: Guo Ren
>
> #define MAP_NR(x) PFN_DOWN((unsigned long)(x) - PAGE_OFFSET - \
> --
> 2.17.1
>
--
Best Regards
Guo Ren
Re: [PATCH 1/4] asm-generic/page.h: apply page shift to PFN instead of VA in pfn_to_virt
On Wed, Jan 31, 2024 at 2:27 PM Yan Zhao wrote:
>
> Apply the page shift to PFN to get physical address for final VA.
> The macro __va should take physical address instead of PFN as input.
>
> Fixes: 2d78057f0dd4 ("asm-generic/page.h: Make pfn accessors static inlines")
> Signed-off-by: Yan Zhao
> ---
> include/asm-generic/page.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/asm-generic/page.h b/include/asm-generic/page.h
> index 9773582fd96e..4f1265207b9a 100644
> --- a/include/asm-generic/page.h
> +++ b/include/asm-generic/page.h
> @@ -81,7 +81,7 @@ static inline unsigned long virt_to_pfn(const void *kaddr)
> #define virt_to_pfn virt_to_pfn
> static inline void *pfn_to_virt(unsigned long pfn)
> {
> - return __va(pfn) << PAGE_SHIFT;
> + return __va(pfn << PAGE_SHIFT);
Oh, that's a terrible bug; Thx for fixing it.
Reviewed-by: Guo Ren
> }
> #define pfn_to_virt pfn_to_virt
>
> --
> 2.17.1
>
--
Best Regards
Guo Ren
Re: [PATCH v9 10/15] x86/sgx: Add EPC reclamation in cgroup try_charge()
> > > Right. When code reaches to here, we already passed reclaim per cgroup. Yes if try_charge() failed we must do pre-cgroup reclaim. > The cgroup may not at or reach limit but system has run out of physical > EPC. > But after try_charge() we can still choose to reclaim from the current group, but not necessarily have to be global, right? I am not sure whether I am missing something, but could you elaborate why we should choose to reclaim from the global?
Re: [PATCH] virtiofs: limit the length of ITER_KVEC dio by max_nopage_rw
On Wed, 3 Jan 2024 at 11:58, Hou Tao wrote: > > From: Hou Tao > > When trying to insert a 10MB kernel module kept in a virtiofs with cache > disabled, the following warning was reported: > > [ cut here ] > WARNING: CPU: 2 PID: 439 at mm/page_alloc.c:4544 .. > Modules linked in: > CPU: 2 PID: 439 Comm: insmod Not tainted 6.7.0-rc7+ #33 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), .. > RIP: 0010:__alloc_pages+0x2c4/0x360 > .. > Call Trace: > >? __warn+0x8f/0x150 >? __alloc_pages+0x2c4/0x360 >__kmalloc_large_node+0x86/0x160 >__kmalloc+0xcd/0x140 >virtio_fs_enqueue_req+0x240/0x6d0 >virtio_fs_wake_pending_and_unlock+0x7f/0x190 >queue_request_and_unlock+0x58/0x70 >fuse_simple_request+0x18b/0x2e0 >fuse_direct_io+0x58a/0x850 >fuse_file_read_iter+0xdb/0x130 >__kernel_read+0xf3/0x260 >kernel_read+0x45/0x60 >kernel_read_file+0x1ad/0x2b0 >init_module_from_file+0x6a/0xe0 >idempotent_init_module+0x179/0x230 >__x64_sys_finit_module+0x5d/0xb0 >do_syscall_64+0x36/0xb0 >entry_SYSCALL_64_after_hwframe+0x6e/0x76 >.. > > ---[ end trace ]--- > > The warning happened as follow. In copy_args_to_argbuf(), virtiofs uses > kmalloc-ed memory as bound buffer for fuse args, but So this seems to be the special case in fuse_get_user_pages() when the read/write requests get a piece of kernel memory. I don't really understand the comment in virtio_fs_enqueue_req(): /* Use a bounce buffer since stack args cannot be mapped */ Stefan, can you explain? What's special about the arg being on the stack? What if the arg is not on the stack (as is probably the case for big args like this)? Do we need the bounce buffer in that case? Thanks, Miklos
Re: [PATCH -next] VDUSE: fix another doc underline warning
On Thu, Feb 22, 2024 at 10:23:41PM -0800, Randy Dunlap wrote:
> Extend the underline for a heading to prevent a documentation
> build warning. Also spell "reconnection" correctly.
>
> Documentation/userspace-api/vduse.rst:236: WARNING: Title underline too short.
> HOW VDUSE devices reconnectoin works
>
>
> Fixes: 2b3fd606c662 ("Documentation: Add reconnect process for VDUSE")
> Signed-off-by: Randy Dunlap
> Cc: Cindy Lu
> Cc: Michael S. Tsirkin
> Cc: Jason Wang
> Cc: Xuan Zhuo
> Cc: virtualizat...@lists.linux.dev
> Cc: Jonathan Corbet
Thanks, I fixed this in my tree already.
> ---
> Documentation/userspace-api/vduse.rst |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff -- a/Documentation/userspace-api/vduse.rst
> b/Documentation/userspace-api/vduse.rst
> --- a/Documentation/userspace-api/vduse.rst
> +++ b/Documentation/userspace-api/vduse.rst
> @@ -232,8 +232,8 @@ able to start the dataplane processing a
>
> For more details on the uAPI, please see include/uapi/linux/vduse.h.
>
> -HOW VDUSE devices reconnectoin works
> -
> +HOW VDUSE devices reconnection works
> +
> 0. Userspace APP checks if the device /dev/vduse/vduse_name exists.
> If it does not exist, need to create the instance.goto step 1
> If it does exist, it means this is a reconnect and goto step 3.
Re: [PATCH] vduse: implement DMA sync callbacks
On Thu, Feb 22, 2024 at 03:29:10PM -0500, Michael S. Tsirkin wrote: > In a sense ... but on the other hand, the "fake DMA" metaphor seems to > work surprisingly well, like in this instance - internal bounce buffer > looks a bit like non-coherent DMA. A way to make this all prettier > would I guess be to actually wrap all of DMA with virtio wrappers which > would all go if () dma_... else vduse_...; or something to this end. A > lot of work for sure, and is it really worth it? if the only crazy > driver is vduse I'd maybe rather keep the crazy hacks local there ... Well, vduse is the only driver that does this hack - we had a few more and we got rid of it. It basically is the only thing preventing us from doing direct calls into the iommu code and compile out dma_ops entirely for non-Xen builds on the common architectures. So yes, I'd really like to see it gone rather sooner than later.
