* Re: [PATCH] sched/topology: Allow EAS without schedutil for artificial Energy Models
From: Lucas Lima @ 2026-06-29 19:07 UTC (permalink / raw)
To: Lukasz Luba
Cc: dietmar.eggemann, rostedt, vincent.guittot, mingo, bsegall,
mgorman, vschneid, kprateek.nayak, corbet, skhan, linux-pm,
linux-doc, linux-kernel, juri.lelli, rafael, viresh.kumar, peterz
In-Reply-To: <7f960dca-2de2-4831-8690-a40fdec848dd@arm.com>
Em seg., 29 de jun. de 2026 às 11:05, Lukasz Luba
<lukasz.luba@arm.com> escreveu:
>
>
>
> On 6/29/26 09:35, Lucas de Lima Nóbrega wrote:
> > EAS currently refuses to enable energy-aware scheduling on a root
> > domain unless schedutil is the active CPUFreq governor for all of its
> > CPUs (cpufreq_ready_for_eas()). This requirement exists to protect the
> > accuracy of the energy estimate: EAS predicts the OPP a CPU will run
> > at from its utilization, which is only meaningful if the active
> > governor actually requests OPPs that way, and schedutil is the only
> > one that does.
> >
> > That requirement does not apply to artificial Energy Models
> > (EM_PERF_DOMAIN_ARTIFICIAL). An artificial EM is built from a
> > get_cost() callback instead of real power numbers, and only encodes a
> > cost ranking between CPUs (e.g. P-cores cost more than E-cores at a
> > given utilization). It never claims to predict real energy use at any
> > specific OPP, so there is no per-OPP accuracy for the governor
> > requirement to protect, regardless of which governor is in control or
> > whether it tracks utilization at all.
> >
> > intel_pstate registers exactly this kind of artificial EM for hybrid
> > (P/E-core) systems without SMT, regardless of whether it operates in
> > active or passive mode. In active mode it never uses schedutil, since
> > HWP picks frequency autonomously, so on these systems EAS never
>
> When frequency is picked autonomously then EAS and energy estimations
> don't make sense IMHO.
>
> Do you have any data from experiments how it runs?
>
> Regards,
> Lukasz
Through my testings, HWP does seem follow an strictly non decreasing
duty-cycle to freq curve, with or without frequency clamping and turbo
boost activated. For me, that feels enough to justify the use of the
simplified energy model already implemented.
^ permalink raw reply
* Re: [PATCH v3] cgroup/cpu: document cpu.stat.local and clarify cpu.stat behavior
From: Tejun Heo @ 2026-06-29 18:55 UTC (permalink / raw)
To: Sun Shaojie
Cc: Michal Koutný, Jonathan Corbet, cui.tao, Johannes Weiner,
Shuah Khan, cgroups, linux-doc, linux-kernel
In-Reply-To: <20260629060636.200118-1-sunshaojie@kylinos.cn>
Hello,
Applied to cgroup/for-7.3.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver
From: Andrew Davis @ 2026-06-29 19:22 UTC (permalink / raw)
To: Shenwei Wang (OSS), Linus Walleij, Bartosz Golaszewski,
Jonathan Corbet, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Mathieu Poirier, Frank Li, Sascha Hauer
Cc: Shuah Khan, linux-gpio@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, Pengutronix Kernel Team,
Fabio Estevam, Shenwei Wang, Peng Fan, devicetree@vger.kernel.org,
linux-remoteproc@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org, dl-linux-imx,
Arnaud POULIQUEN, b-padhi@ti.com, Andrew Lunn,
Bartosz Golaszewski
In-Reply-To: <DB8PR04MB712990DF9806BBCF36B1076DC8E82@DB8PR04MB7129.eurprd04.prod.outlook.com>
On 6/29/26 1:26 PM, Shenwei Wang (OSS) wrote:
>
>
>> -----Original Message-----
>> From: Andrew Davis <afd@ti.com>
>> Sent: Thursday, June 25, 2026 3:32 PM
>
> ...
>> Subject: Re: [PATCH v14 4/5] gpio: rpmsg: add generic rpmsg GPIO driver
>>> + Say yes here to support the generic GPIO functions over the RPMSG
>>> + bus. Currently supported devices: i.MX7ULP, i.MX8ULP, i.MX8x, and
>>> + i.MX9x.
>>
>> The support would depend on if the right firmware is loaded/running on the given
>> remote core. Also if you want to make this generic, then any vendor should be
>> able to make a firmware that implements this protocol and make use of this
>> driver.
>> Suggest dropping this NXP specific device list.
>>
>
> Agree.
>
>>> +
>>> + If unsure, say N.
>>> +
>>> +endmenu
>>> +
>>> menu "SPI GPIO expanders"
>>> depends on SPI_MASTER
>>>
>>> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index
>>> b267598b517d..ee75c0e65b8b 100644
>>> --- a/drivers/gpio/Makefile
>>> +++ b/drivers/gpio/Makefile
>>> @@ -157,6 +157,7 @@ obj-$(CONFIG_GPIO_RDC321X) += gpio-
>
> ...
>>> +
>>> +static int rpmsg_gpio_channel_probe(struct rpmsg_device *rpdev) {
>>> + struct device *dev = &rpdev->dev;
>>> + struct device_node *np;
>>> + const char *rproc_name;
>>> + int idx;
>>> +
>>> + idx = rpmsg_get_gpio_index(rpdev->id.name, CHAN_NAME_PREFIX);
>>> + if (idx < 0)
>>> + return -EINVAL;
>>> +
>>> + if (!dev->of_node) {
>>> + np = rpmsg_get_channel_ofnode(rpdev, GPIO_COMPAT_STR, idx);
>>> + if (!np)
>>> + return -ENODEV;
>>
>> This seems to imply that DT nodes are required. RPMSG is a discoverable bus
>> with a nameservice that can bind/probe new devices. While then optionally
>> binding to a DT node when available so sub-devices can be described in DT is fine,
>> I don't see why it should be required.
>>
>
> First, a GPIO node typically acts as a provider for other devices.
Not necessarily, there is a userspace API for interacting with GPIOs.
And there are ways to get/attach GPIO lines to other devices without DT.
> Second, by requiring a DT node, we can ensure that only explicitly enabled GPIO resources are managed and accessible.
Not sure I follow here, you have a firmware that provides GPIOs to Linux,
Linux should register those with the GPIO framework. Not sure why DT
is required to be involved. Some systems don't do DT, but they have
firmware and GPIOs.
I'm not saying if the system does use DT and has a node specifically
for this firmware/gpio then we shouldn't bind to that and use it,
just questioning making that "required".
Andrew
>
>>> +static struct rpmsg_driver rpmsg_gpio_channel_client = {
>>> + .callback = rpmsg_gpio_channel_callback,
>>> + .id_table = rpmsg_gpio_channel_id_table,
>>> + .probe = rpmsg_gpio_channel_probe,
>>> + .drv = {
>>> + .name = KBUILD_MODNAME,
>>> + .of_match_table = rpmsg_gpio_dt_ids,
>>
>> Does this line actually do anything anymore? Maybe it did when this was a
>> platform_driver, but this is a rpmsg_driver and will probe though .id_table
>> matches.
>>
>
> Yes, it can be removed because the driver will find the dt node on its own.
>
> Thanks,
> Shenwei
>
>> Andrew
>>
>>> + },
>>> +};
>>> +module_rpmsg_driver(rpmsg_gpio_channel_client);
>>> +
>>> +MODULE_AUTHOR("Shenwei Wang <shenwei.wang@nxp.com>");
>>> +MODULE_DESCRIPTION("generic rpmsg gpio driver");
>>> +MODULE_LICENSE("GPL");
>>
>
^ permalink raw reply
* Re: [PATCH v2 1/9] dt-bindings: adm1275: ROHM BD12780 hot-swap controller
From: Guenter Roeck @ 2026-06-29 20:44 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Matti Vaittinen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan,
Wensheng Wang, Ashish Yadav, Vasileios Amoiridis, Kim Seer Paller,
ChiShih Tsai, Chris Packham, Robert Coulson, linux-hwmon,
devicetree, linux-kernel, linux-doc
In-Reply-To: <2b7d5bb8cba773d0bba1d6779f0e6daa6a40eed4.1782458224.git.mazziesaccount@gmail.com>
On Fri, Jun 26, 2026 at 10:21:42AM +0300, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> Support ROHM BD12780 and BD12780A hot-swap controllers, which are largely
> compatible with the Analog Devices adm1278. Main difference between
> the BD12780 and the BD12780A is, that the BD12780 has one I2C address
> configuration pin more (ADDR3) than the BD12780A.
>
> Introduce own compatibles for both variants but require the BD12780A to
> always have the BD12780 as a fall-back.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
Applied.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v2 2/9] hwmon: adm1275: Prevent reading uninitialized stack
From: Guenter Roeck @ 2026-06-29 20:45 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Matti Vaittinen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan,
Wensheng Wang, Ashish Yadav, Vasileios Amoiridis, Kim Seer Paller,
ChiShih Tsai, Chris Packham, Robert Coulson, linux-hwmon,
devicetree, linux-kernel, linux-doc
In-Reply-To: <c8ad38e0cdb347261c6245de2b7965e747f28d22.1782458224.git.mazziesaccount@gmail.com>
On Fri, Jun 26, 2026 at 10:22:04AM +0300, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> While adding support for the ROHM BD127X0 hot-swap controllers, sashiko
> reported an error in device-name comparison, which can lead to reading
> uninitialized stack memory.
>
> Quoting Sashiko:
>
> This is a pre-existing issue, but I noticed that just before this block in
> adm1275_probe(), there might be an out-of-bounds stack read:
>
> ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, block_buffer);
> if (ret < 0) { ... }
> for (mid = adm1275_id; mid->name[0]; mid++) {
> if (!strncasecmp(mid->name, block_buffer, strlen(mid->name)))
> break;
> }
>
> Since i2c_smbus_read_block_data() reads up to 32 bytes into the
> uninitialized stack array block_buffer without appending a null
> terminator, strncasecmp() could read past the valid bytes returned in ret.
>
> For example, if the device returns a shorter string like "adm12", checking
> it against "adm1275" up to the length of "adm1275" will continue reading
> into uninitialized stack bounds.
>
> Prevent reading uninitialized memory by zeroing the stack array.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> Fixes: 87102808d039 ("hwmon: (pmbus/adm1275) Validate device ID")
Applied.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v2 3/9] hwmon: adm1275: Detect coefficient overflow
From: Guenter Roeck @ 2026-06-29 20:46 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Matti Vaittinen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan,
Wensheng Wang, Ashish Yadav, Vasileios Amoiridis, Kim Seer Paller,
ChiShih Tsai, Chris Packham, Robert Coulson, linux-hwmon,
devicetree, linux-kernel, linux-doc
In-Reply-To: <d9e3320dbd62e094ff89598cb3aac5b5e716f9e7.1782458224.git.mazziesaccount@gmail.com>
On Fri, Jun 26, 2026 at 10:23:58AM +0300, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> Sashiko detected potential coefficient overflow if large shunt resistor
> is used. When going unnoticed it can cause "drastically incorrect
> telemetry scaling factors" as Sashiko put it.
>
> I am not convinced such "drastically incorrect telemetry scaling
> factors" could have gone unnoticed, so I suspect such large shunt
> resistors aren't really used. Well, it shouldn't hurt to detect the
> error and abort the probe before Really Wrong current / power -values
> are reported to user by the hwmon.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> ---
> Revision history:
> v1 => v2:
> - New patch
>
> This patch returns -EOVERFLOW with an error print if overflow is
> detected. IF there really are systems where the overflow truly occurs,
> then this change will cause the probe to fail - which might hurt the
> boot process. It might be safer to only print the warning. One could
> also try changing the order of the shunt resistor value division (/1000)
> and the multiplication and see if overflow goes away - but it'll be
> somewhat more complex then. Hence, I just decided to error-out if this
> happens, and leave this for the people facing the real overflow to fix
> (if needed)... It's still fair to mention this might cause issues.
Let's see if it happens.
Applied.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v2 4/9] hwmon: adm1275: Support module auto-loading
From: Guenter Roeck @ 2026-06-29 20:49 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Matti Vaittinen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan,
Wensheng Wang, Ashish Yadav, Vasileios Amoiridis, Kim Seer Paller,
ChiShih Tsai, Chris Packham, Robert Coulson, linux-hwmon,
devicetree, linux-kernel, linux-doc
In-Reply-To: <9eca6831f9fe2d781bb88337397c39b10e36f5c6.1782458224.git.mazziesaccount@gmail.com>
On Fri, Jun 26, 2026 at 10:24:21AM +0300, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> Populating the i2c_device_id -table is not enough to make the
> driver module automatically load when device-tree node for the
> power-monitor is parsed at boot.
>
> Adding the of_device_id tables causes the driver module to be
> automatically load at boot. Testing has been done with rather old Debian
> system.
>
> When inspecting the generated module-aliases with the insmod, following
> entries seem to be the difference:
>
> alias: of:N*T*Cadi,adm1075C*
> alias: of:N*T*Cadi,adm1075
>
> I suspect these are required for the module loading to work.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Applied.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v2 5/9] doc: Add ROHM BD12780 and BD12780A
From: Guenter Roeck @ 2026-06-29 20:50 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Matti Vaittinen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan,
Wensheng Wang, Ashish Yadav, Vasileios Amoiridis, Kim Seer Paller,
ChiShih Tsai, Chris Packham, Robert Coulson, linux-hwmon,
devicetree, linux-kernel, linux-doc
In-Reply-To: <5ff4a4a7a939cd642522fde407b5c9fa123a7089.1782458224.git.mazziesaccount@gmail.com>
On Fri, Jun 26, 2026 at 10:24:38AM +0300, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> Add the ROHM BD12780 and the BD12780A to the list of the ICs supported by
> the adm1275 driver.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Applied.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v2 6/9] hwmon: adm1275: Support ROHM BD12780
From: Guenter Roeck @ 2026-06-29 20:55 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Matti Vaittinen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan,
Wensheng Wang, Ashish Yadav, Vasileios Amoiridis, Kim Seer Paller,
ChiShih Tsai, Chris Packham, Robert Coulson, linux-hwmon,
devicetree, linux-kernel, linux-doc
In-Reply-To: <e1e6e6218c08b562311356ef9c57378d32c26b08.1782458224.git.mazziesaccount@gmail.com>
On Fri, Jun 26, 2026 at 10:24:55AM +0300, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> ROHM BD12780 and BD12780A are hot-swap controllers. They are largely
> similar to Analog Devices ADM1278. Besides the ID registers and some
> added functionality, the BD12780 and BD12780A mark PMON_CONFIG bits
> [15:14] as reserved. Hence TSFILT setting must be omitted on these ICs.
>
> The BD12780 has 3 pins usable for configuring the I2C address. The
> BD12780A lists the ADDR3-pin as "not connect".
>
> Support ROHM BD12780 and BD12780A controllers.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Applied.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v2 8/9] doc: adm1275: Add ROHM BD12790
From: Guenter Roeck @ 2026-06-29 20:56 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Matti Vaittinen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan,
Wensheng Wang, Ashish Yadav, Vasileios Amoiridis, Kim Seer Paller,
ChiShih Tsai, Chris Packham, Robert Coulson, linux-hwmon,
devicetree, linux-kernel, linux-doc
In-Reply-To: <9bafcfe37190b5a3925e8dcfa3b59f8d240234ba.1782458224.git.mazziesaccount@gmail.com>
On Fri, Jun 26, 2026 at 10:25:33AM +0300, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> Add the ROHM BD12790 to the list of the ICs supported by the adm1275
> driver.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
Applied.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v2 7/9] dt-bindings: adm1275: ROHM BD12790 hot-swap controller
From: Guenter Roeck @ 2026-06-29 20:56 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Matti Vaittinen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan,
Wensheng Wang, Ashish Yadav, Vasileios Amoiridis, Kim Seer Paller,
ChiShih Tsai, Chris Packham, Robert Coulson, linux-hwmon,
devicetree, linux-kernel, linux-doc
In-Reply-To: <753a6ded3e1aff1aaba7d932f0a883f789d9dfaa.1782458224.git.mazziesaccount@gmail.com>
On Fri, Jun 26, 2026 at 10:25:13AM +0300, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> Support ROHM BD12790 hot-swap controller which is largely compatible
> with the Analog Devices adm1272.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
Applied.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v2 9/9] hwmon: adm1275: Support ROHM BD12790
From: Guenter Roeck @ 2026-06-29 21:08 UTC (permalink / raw)
To: Matti Vaittinen
Cc: Matti Vaittinen, Matti Vaittinen, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan,
Wensheng Wang, Ashish Yadav, Vasileios Amoiridis, Kim Seer Paller,
ChiShih Tsai, Chris Packham, Robert Coulson, linux-hwmon,
devicetree, linux-kernel, linux-doc
In-Reply-To: <b209c1b47712b69f17b52cfd7a7a38ed76024ca7.1782458224.git.mazziesaccount@gmail.com>
On Fri, Jun 26, 2026 at 10:26:02AM +0300, Matti Vaittinen wrote:
> From: Matti Vaittinen <mazziesaccount@gmail.com>
>
> Add support for ROHM BD12790 hot-swap controller which is largely
> similar to Analog Devices adm1272.
>
> The BD12790 uses the same selectable 60V/100V voltage ranges and
> 15mV/30mV current-sense ranges as the ADM1272, and the same VRANGE
> (bit 5) and IRANGE (bit 0) layout in PMON_CONFIG. It therefore uses
> a dedicated coefficient table that mirrors adm1272_coefficients, with
> the following differences derived from BD12790 datasheet Table 1 (p.18):
> - power 60V/30mV: m=17560 (vs. 17561)
> - power 100V/30mV: m=10536 (vs. 10535)
> - temperature: b=31880 (vs. 31871, reflecting T[11:0] = 4.2*T + 3188)
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> Assisted-by: GitHub Copilot:claude-sonnet-4.6
Applied, after fixing the comment about BD12790 coefficients as suggested.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH] sched/topology: Allow EAS without schedutil for artificial Energy Models
From: Lucas Lima @ 2026-06-29 21:12 UTC (permalink / raw)
To: Rafael J. Wysocki (Intel)
Cc: viresh.kumar, mingo, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
kprateek.nayak, corbet, skhan, linux-pm, linux-doc, linux-kernel
In-Reply-To: <CAJZ5v0iVD90XPsWgT8B+fw9vmFRZTVL7MasPP-=Ci0OdUmNR=A@mail.gmail.com>
Em seg., 29 de jun. de 2026 às 12:16, Rafael J. Wysocki (Intel)
<rafael@kernel.org> escreveu:
>
> On Mon, Jun 29, 2026 at 10:36 AM Lucas de Lima Nóbrega
> <lucaslnobrega38@gmail.com> wrote:
> >
> > EAS currently refuses to enable energy-aware scheduling on a root
> > domain unless schedutil is the active CPUFreq governor for all of its
> > CPUs (cpufreq_ready_for_eas()). This requirement exists to protect the
> > accuracy of the energy estimate: EAS predicts the OPP a CPU will run
> > at from its utilization, which is only meaningful if the active
> > governor actually requests OPPs that way, and schedutil is the only
> > one that does.
> >
> > That requirement does not apply to artificial Energy Models
> > (EM_PERF_DOMAIN_ARTIFICIAL). An artificial EM is built from a
> > get_cost() callback instead of real power numbers, and only encodes a
> > cost ranking between CPUs (e.g. P-cores cost more than E-cores at a
> > given utilization). It never claims to predict real energy use at any
> > specific OPP, so there is no per-OPP accuracy for the governor
> > requirement to protect, regardless of which governor is in control or
> > whether it tracks utilization at all.
>
> But it is still about comparing the cost of running on different CPUs
> at different performance levels.
>
> For instance, say the scale-invariant utilization of a task is 256 and
> it can run either by itself on a P-core, or with another task whose
> utilization is 128 on an E-core, and say the P-core's and E-core's
> capacity is 1024 and 512, respectively.
>
> Say the cost function tells EAS that running a P-core at 1/4 of the
> capacity is cheaper than running an E-core at 3/4 capacity, so it will
> pick up the P-core to run that task, but if cpufreq ramps up the
> frequency of the P-core to the max when the task gets to it, it may
> actually turn out to be more expensive.
>
> This means that EAS still has an expectation regarding cpufreq which
> is that it will generally tend to run tasks at the performance level
> corresponding to the sum of their scale-invariant utilization at least
> roughly.
>
> IIUC this actually has nothing to do with whether or not the energy
> model used by EAS is artificial. The schedutil requirement is about
> choosing a performance level proportional to the utilization (which
> schedutil generally tends to do by design).
You're right, and I want to walk back the "artificial EM doesn't need
this" framing entirely -- it doesn't survive your example. What I want
to argue instead is narrower: that even though intel_pstate active
mode tracks demand much more weakly than schedutil, the specific
conclusion this simplified EM's cost ranking relies on (E-cores cost
less than P-cores at matched conditions) still holds up against
measured energy, and that's a different, more modest claim than "OPP
tracks utilization closely enough for per-bin accuracy."
I measured the actual frequency behavior on this test machine (one
P-core, one E-core, isolated, stress-ng --cpu-load duty cycles at
20/40/60/80/100%, turbostat Bzy_MHz = average frequency only during
the busy portion of each cycle) under three regimes:
20% 40% 60% 80% 100% span
passive+schedutil P 2523 2879 3786 4537 4567 2044
E 2335 2416 2574 3070 3399 1064
active EPP=balance P 2225 2285 2497 2646 2778 553
E 2101 2215 2375 2462 2555 454
active EPP=perf P 4483 4519 4496 4537 4564 81
E 3364 3377 3380 3387 3399 35
It is visible now that intel_pstate active
mode does *not* track demand anywhere near as tightly as schedutil,
and i don't think that claim survives scrutiny, dropping it.
What does survive, I think, is narrower: E-cores measured consistently
cheaper per unit of completed work than P-cores, across every matched-
parallelism configuration I tested (data below), regardless of which
exact OPP HWP autonomously picked underneath. I don't have data on
idle-state residency to know truly whether the race-to-idle behavior under
EPP=performance recovers any of that gap through deeper C-states --
that's an open question I haven't tested.
1 core alone: P 7.27 J/unit E 6.25 J/unit (P +16%)
1 core, packed x2: P 7.24 J/unit E 6.13 J/unit (P +18%)
2 cores, spread: P 4.84 J/unit E 3.82 J/unit (P +27%)
P consistently costs more than E for the same completed work at every
matched parallelism level I tried. Separately, I also measured that
spreading work across more E-cores is itself far more efficient than
packing it onto fewer (8 E-cores spread: 1.74 J/unit vs the same total
work packed onto 1 E-core: 6.10 J/unit. In fact, this is the most
efficient placement --
even better than global spreading) -- I also have traced
find_energy_efficient_cpu()
produced spread placement in practice under this patch with real tasks,
and it roughly does follow this heavy preference for E cores during light load.
Note: P cores occasionally seem to spike, likely due to misfit tasks
which are larger
than E core capacity when nosmt=force is active (512). To place E
cores capacities
at half of P cores' feels weird, as the vast majority of workloads enjoy only a
40-60% performance disparity between them both (the outliers observed are
mostly float point heavy tasks, software ipc class 2).
>
> > intel_pstate registers exactly this kind of artificial EM for hybrid
> > (P/E-core) systems without SMT, regardless of whether it operates in
> > active or passive mode. In active mode it never uses schedutil, since
> > HWP picks frequency autonomously, so on these systems EAS never
> > engages even though SD_ASYM_CPUCAPACITY, frequency invariance and the
> > EM are all in place: find_energy_efficient_cpu() is never reached
> > because is_rd_overutilized() is hardcoded to true whenever
> > sched_energy_enabled() is false. cppc_cpufreq registers the same kind
> > of ranking-only artificial EM and is affected the same way with any
> > non-schedutil governor.
> >
> > Allow EAS to be enabled when every CPU's EM in the root domain is
> > artificial, even when schedutil is not the active governor.
> >
> > Tested on a Raptor Lake-P laptop with nosmt=force and intel_pstate in
> > active/HWP mode: find_energy_efficient_cpu() was never called before
> > this change (confirmed via the sched_overutilized_tp tracepoint and
> > ftrace) and is exercised as expected afterwards.
>
> If this is about allowing EAS to work with intel_pstate running in the
> active mode, you may argue that what the processor firmware is doing
> when intel_pstate runs in the active mode is not much different from
> what schedutil would do. So a driver implementing an internal
> governor (that is, using the .set_policy() callback) would need to
> declare that its internal governor is as good as schedutil from EAS'
> perspective and so it will pass the "cpufreq readiness" check.
Given the data above, I don't think I can honestly word that
declaration as "as good as schedutil" -- it isn't, by a factor of
2-25x depending on EPP. If a flag like this still makes sense, I'd
want its justification to say something narrower: "this driver's
internal governor, combined with this EM's coarse type-based ranking,
still produces correct placement decisions in practice" rather than
claiming OPP-tracking parity. I'm not sure if that's a distinction
that belongs in the flag's contract itself, or just in this
patch's commit message -- happy to go either way, or to test more
if that would help decide.
>
> > Signed-off-by: Lucas de Lima Nóbrega <lucaslnobrega38@gmail.com>
> > ---
> > Documentation/admin-guide/pm/intel_pstate.rst | 9 ++++--
> > Documentation/scheduler/sched-energy.rst | 7 ++++-
> > kernel/sched/topology.c | 28 +++++++++++++++++--
> > 3 files changed, 38 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/admin-guide/pm/intel_pstate.rst b/Documentation/admin-guide/pm/intel_pstate.rst
> > index 25fe5d88f..c8fef1e60 100644
> > --- a/Documentation/admin-guide/pm/intel_pstate.rst
> > +++ b/Documentation/admin-guide/pm/intel_pstate.rst
> > @@ -409,13 +409,16 @@ Energy-Aware Scheduling Support
> > If ``CONFIG_ENERGY_MODEL`` has been set during kernel configuration and
> > ``intel_pstate`` runs on a hybrid processor without SMT, in addition to enabling
> > :ref:`CAS` it registers an Energy Model for the processor. This allows the
> > -Energy-Aware Scheduling (EAS) support to be enabled in the CPU scheduler if
> > -``schedutil`` is used as the ``CPUFreq`` governor which requires ``intel_pstate``
> > -to operate in the :ref:`passive mode <passive_mode>`.
> > +Energy-Aware Scheduling (EAS) support to be enabled in the CPU scheduler.
> >
> > The Energy Model registered by ``intel_pstate`` is artificial (that is, it is
> > based on abstract cost values and it does not include any real power numbers)
> > and it is relatively simple to avoid unnecessary computations in the scheduler.
> > +Because of that, EAS does not require ``schedutil`` to be used as the
> > +``CPUFreq`` governor in this case: the cost ranking it relies on does not
> > +depend on the governor tracking utilization when requesting frequencies, so
> > +EAS works the same way regardless of whether ``intel_pstate`` operates in the
> > +active or in the :ref:`passive mode <passive_mode>`.
> > There is a performance domain in it for every CPU in the system and the cost
> > values for these performance domains have been chosen so that running a task on
> > a less performant (small) CPU appears to be always cheaper than running that
> > diff --git a/Documentation/scheduler/sched-energy.rst b/Documentation/scheduler/sched-energy.rst
> > index 4e47aaf10..c23ca226d 100644
> > --- a/Documentation/scheduler/sched-energy.rst
> > +++ b/Documentation/scheduler/sched-energy.rst
> > @@ -379,7 +379,12 @@ Consequently, the only sane governor to use together with EAS is schedutil,
> > because it is the only one providing some degree of consistency between
> > frequency requests and energy predictions.
> >
> > -Using EAS with any other governor than schedutil is not supported.
> > +Using EAS with any other governor than schedutil is not supported, unless the
> > +EM in use is artificial (see EM_PERF_DOMAIN_ARTIFICIAL). An artificial EM only
> > +encodes a cost ranking between CPUs/OPPs instead of a real power table, so it
> > +does not make any claim about energy use at a specific OPP and its conclusions
> > +do not depend on the governor actually tracking utilization when requesting
> > +frequencies.
> >
> >
> > 6.5 Scale-invariant utilization signals
> > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > index 5847b83d9..124a4bb4d 100644
> > --- a/kernel/sched/topology.c
> > +++ b/kernel/sched/topology.c
> > @@ -212,6 +212,27 @@ static unsigned int sysctl_sched_energy_aware = 1;
> > static DEFINE_MUTEX(sched_energy_mutex);
> > static bool sched_energy_update;
> >
> > +/*
> > + * An artificial EM (see EM_PERF_DOMAIN_ARTIFICIAL) only encodes a cost
> > + * ranking between CPUs and does not claim to predict energy use at any
> > + * particular OPP. Unlike a real power-based EM, its conclusions do not
> > + * rely on the active governor tracking utilization when selecting
> > + * frequencies, so the schedutil requirement below does not apply to it.
> > + */
> > +static bool perf_domains_are_artificial(const struct cpumask *cpu_mask)
> > +{
> > + int i;
> > +
> > + for_each_cpu(i, cpu_mask) {
> > + struct em_perf_domain *pd = em_cpu_get(i);
>
> I would do
>
> if (!pd)
> continue;
>
> here because the CPUs without a PD simply don't matter.
That's fair. I will be updating the code to ignore cpus with no perf
domain. I also want to discuss whether or not is it worth it to
aggregate E clusters inside the same perf domain, as they share the
same L2 cache and
migrations are likely easier.
>
> Also, is any synchronization needed for this?
No additional sync besides what is already in use today. In fact, this
very pointer is dereferenced the same way in other paths of the
kernel.
^ permalink raw reply
* Re: [PATCH v4] hwmon: add a driver for the temp/voltage sensor on PolarFire SoC
From: Guenter Roeck @ 2026-06-29 21:20 UTC (permalink / raw)
To: Conor Dooley
Cc: linux-hwmon, Lars Randers, Conor Dooley, Jonathan Corbet,
Shuah Khan, Daire McNamara, linux-doc, linux-kernel, linux-riscv,
Valentina.FernandezAlanis
In-Reply-To: <20260629-wriggle-headscarf-c85a4070dddb@spud>
On Mon, Jun 29, 2026 at 10:30:59AM +0100, Conor Dooley wrote:
> From: Lars Randers <lranders@mail.dk>
>
> Add a driver for the temperature and voltage sensors on PolarFire SoC.
> The temperature reports how hot the die is, and the voltages are the
> SoC's 1.05, 1.8 and 2.5 volt rails respectively.
>
> The hardware supports alarms in theory, but there is an erratum that
> prevents clearing them once triggered, so no support is added for them.
>
> The hardware measures voltage with 16 bits, of which 1 is a sign bit and
> the remainder holds the voltage as a fixed point integer value. It's
> improbable that the hardware will work if the voltages are negative, so
> the driver ignores the sign bits.
>
> There's no dt support etc here because this is the child of a simple-mfd
> syscon.
>
> Signed-off-by: Lars Randers <lranders@mail.dk>
> Co-developed-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
Applied.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH net-deletions] net: remove ax25 and amateur radio (hamradio) subsystem
From: Jakub Kicinski @ 2026-06-29 22:29 UTC (permalink / raw)
To: Jiri Slaby
Cc: davem, netdev, edumazet, pabeni, andrew+netdev, horms, corbet,
skhan, federico.vaga, carlos.bilbao, avadhut.naik, alexs,
si.yanteng, dzm91, 2023002089, tsbogend, dsahern, jani.nikula,
mchehab+huawei, gregkh, tytso, herbert, ebiggers, johannes.berg,
geert, pablo, tglx, mashiro.chen, mingo, dqfext, jreuter, sdf,
pkshih, enelsonmoore, mkl, toke, kees, crossd, jlayton,
wangliang74, aha310510, takamitz, kuniyu, linux-doc, linux-mips
In-Reply-To: <8458cf47-43f3-4328-bc6a-9aac31f1acf4@kernel.org>
On Mon, 29 Jun 2026 07:24:51 +0200 Jiri Slaby wrote:
> And net-tools are broken by the uapi/linux/rose.h removal:
> rose.c:39:10: fatal error: linux/rose.h: No such file or directory
> at:
> https://github.com/ecki/net-tools/blob/2ab3c5efdb5c220bc9a649fded56c361136bff1a/lib/rose.c#L39
Looks like the code for rose is already well separated from the rest.
Should we just ask net-tools to delete it? Or do you feel strongly
about restoring the now-defunct header?
^ permalink raw reply
* Re: [PATCH v13 04/11] perf/probe: Ignore comment lines in dynamic_events/kprobe_events file
From: Masami Hiramatsu @ 2026-06-29 22:32 UTC (permalink / raw)
To: Namhyung Kim, Arnaldo Carvalho de Melo
Cc: Masami Hiramatsu (Google), Steven Rostedt, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, linux-kernel, linux-trace-kernel,
linux-doc, linux-kselftest
In-Reply-To: <178271361825.1176915.16095297120719039761.stgit@devnote2>
Hi Arnaldo, Namhyung,
I forgot to CC this. Can I pick this patch via linux-trace tree,
or would you pick this?
This is a part of typecast series [1] only for debugging.
[1] https://lore.kernel.org/all/178271361825.1176915.16095297120719039761.stgit@devnote2/
Thanks,
On Mon, 29 Jun 2026 15:13:38 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Since dynamic_events/kprobe_events files show the fetcharg debug
> information as comment lines, its reader needs to ignore it.
>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> tools/perf/util/probe-file.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index 4032572cbf55..4d12693a83b3 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -197,6 +197,8 @@ struct strlist *probe_file__get_rawlist(int fd)
> idx = strlen(p) - 1;
> if (p[idx] == '\n')
> p[idx] = '\0';
> + if (buf[0] == '#')
> + continue;
> ret = strlist__add(sl, buf);
> if (ret < 0) {
> pr_debug("strlist__add failed (%d)\n", ret);
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next] Documentation: networking: Add a test plan for ethtool pause validation
From: Jakub Kicinski @ 2026-06-29 22:40 UTC (permalink / raw)
To: Maxime Chevallier
Cc: Andrew Lunn, davem, Eric Dumazet, Paolo Abeni, Simon Horman,
Russell King, Heiner Kallweit, Jonathan Corbet, Shuah Khan,
Oleksij Rempel, Vladimir Oltean, Florian Fainelli,
thomas.petazzoni, netdev, linux-kernel, linux-doc
In-Reply-To: <01e3d32b-6d50-4179-8e2f-25cdf8ff6c32@bootlin.com>
On Mon, 29 Jun 2026 17:24:51 +0200 Maxime Chevallier wrote:
> On 6/28/26 01:46, Andrew Lunn wrote:
> > On Sat, Jun 27, 2026 at 02:30:28PM -0700, Jakub Kicinski wrote:
> >> The common way of checking prereqs in the tests is to call a function
> >> called require_xyz() which then raises a skip. At a quick glance - the
> >> rss_api and xdp_metadata are good tests to get a sense of the usual format.
> >
> > The counter example is the ksft_disruptive() decorator.
> >
> > Pythons own unittest framework makes use of decorators to skip
> > tests. Its the Pythonic way.
Problem is vast majority of our developers do not know "Pythonic" ways.
And bash tests for HW are an absolute pile of impossible to debug
dookie. Half of the time bash doesn't print anything when test fails,
so good luck figuring out what happened on someone else's setup 10%
of the time :|
I'm hoping to strike a balance with keeping Python relatively dumb,
and pulling as many people as possible away form bash.
> So maybe in the end, we can try to have something a bit less python-y, while still
> using extensive documentation using sphynx doc format ?
>
> Let me send a V2 with the full test list, we'll see how much scaffolding
> we can build for ethtool testing, and how. I suspect that running/skipping based on
> the device's capabilities is going to be used throughout lots of tests
> beyond pause.
>
> For now the important part is to get that test list right, and iterate on the
> test implementation once we agree on what to test, why and how.
Maybe we should step back and figure out the full story for the
self-documentation thing. The good tests under tools...drivers/net
already have module and test-case level doc. So we can come up with
a way of extracting that and weaving that into NIPA? That way we are
all on the same page?
How should the test info be presented? I think it'd fit best in here:
https://netdev.bots.linux.dev/devices.html ? Or do you have something
else in mind?
^ permalink raw reply
* Re: [PATCH RFC v6 0/5] iio: add Open Sensor Fusion IIO driver
From: Jonathan Cameron @ 2026-06-29 22:58 UTC (permalink / raw)
To: David Lechner
Cc: Jinseob Kim, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nuno Sá, Andy Shevchenko, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-doc, linux-kernel
In-Reply-To: <9b179166-3057-4867-be04-b071a809f70f@baylibre.com>
On Mon, 29 Jun 2026 10:25:38 -0500
David Lechner <dlechner@baylibre.com> wrote:
> On 6/28/26 2:13 PM, Jinseob Kim wrote:
> > Open Sensor Fusion (OSF) devices expose a UART/serdev host interface
> > for a sensor aggregation hub. This RFC adds a Linux IIO driver that
> > parses OSF frames and creates IIO devices at runtime from capability
> > reports provided by the device firmware.
> >
> > When the corresponding capabilities are reported, the driver exposes
> > accelerometer, gyroscope, magnetometer, and temperature data as IIO
> > devices named osf-accel, osf-gyro, osf-magn, and osf-temp.
> >
> > This remains RFC while the binding, protocol subset, runtime discovery
> > model, and driver-facing ABI are reviewed.
>
> If you are just looking for review and don't have specific questions,
> then it is time to drop the RFC.
>
It isn't uncommon to keep RFC tags when there is a related spec
that isn't finalised yet. Basically they act as a 'don't merge this'
marker. However I can keep track of this one without it given we
don't have that happen a lot in IIO. Please do make sure that you
keep that bit of the cover letter, Jinseob!
Jonathan
^ permalink raw reply
* Re: [PATCH RFC v6 2/5] Documentation: iio: add Open Sensor Fusion driver overview
From: Jonathan Cameron @ 2026-06-29 23:02 UTC (permalink / raw)
To: Jinseob Kim
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
Nuno Sá, Andy Shevchenko, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-doc, linux-kernel
In-Reply-To: <20260628191337.937-3-kimjinseob88@gmail.com>
On Mon, 29 Jun 2026 04:13:34 +0900
Jinseob Kim <kimjinseob88@gmail.com> wrote:
> Document the Linux IIO mapping for Open Sensor Fusion devices.
>
> The overview explains that sensor channels are discovered at runtime
> from mandatory capability reports. It also documents that OSF0 is a
> wire-format detail and that protocol_major and protocol_minor carry
> protocol compatibility information.
>
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
One small thing inline. Otherwise looks good to me!
Jonathan
> diff --git a/Documentation/iio/open-sensor-fusion.rst b/Documentation/iio/open-sensor-fusion.rst
> new file mode 100644
> index 000000000..832901f5e
> --- /dev/null
> +++ b/Documentation/iio/open-sensor-fusion.rst
> +Timestamps
> +----------
> +
> +OSF frames include a device-side ``timestamp_us`` field. Buffered IIO samples use
> +an IIO timestamp captured on the host when samples are pushed to IIO buffers.
> +The initial driver does not correlate the device timestamp with the host IIO
> +clock.
Really small thing but I would avoid talking about 'initial' driver.
It can cause confusion about when something is true and when it is not
as we may have multiple non 'initial' drivers before you get to changing
this handling. So just remove that word.
^ permalink raw reply
* Re: [PATCH RFC v6 4/5] iio: osf: add authenticated stream parser
From: Jonathan Cameron @ 2026-06-29 23:06 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jinseob Kim, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
David Lechner, Nuno Sá, Andy Shevchenko, Jonathan Corbet,
Shuah Khan, linux-iio, devicetree, linux-doc, linux-kernel
In-Reply-To: <akJ818gkVzLN7HqU@ashevche-desk.local>
On Mon, 29 Jun 2026 17:10:31 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Mon, Jun 29, 2026 at 04:13:36AM +0900, Jinseob Kim wrote:
> > Add a UART byte-stream parser for Open Sensor Fusion frames.
> >
> > The parser searches for the OSF0 wire magic, keeps partial frames
> > buffered, checks header length and payload bounds, and passes complete
> > candidate frames to the core decoder.
> >
> > Rejected candidate frames drop only the current head byte before
> > resynchronizing, so a corrupted unauthenticated payload length cannot
> > make the parser skip later valid frames.
>
> ...
>
> > +#define OSF_STREAM_MAGIC_LEN 4
> > +#define OSF_STREAM_MAX_PAYLOAD_LEN \
> > + (OSF_STREAM_MAX_FRAME_LEN - OSF_FRAME_HEADER_LEN - OSF_FRAME_CRC_LEN)
> > +
> > +static const u8 osf_stream_magic[OSF_STREAM_MAGIC_LEN] = {
> > + 'O', 'S', 'F', '0',
> > +};
>
> You have already this in the header (as FourCC), use that.
>
> ...
>
> > +static size_t osf_stream_discard_to_magic(struct osf_stream *stream)
> > +{
> > + size_t old_len = stream->len;
> > + size_t match_len;
>
> > + size_t i;
> > +
> > + for (i = 0; i < stream->len; i++) {
>
> for (size_t i = 0; i < stream->len; i++) {
>
> > + match_len = stream->len - i;
> > + if (match_len > OSF_STREAM_MAGIC_LEN)
> > + match_len = OSF_STREAM_MAGIC_LEN;
> > +
> > + if (osf_stream_magic_match(stream->buf + i, match_len)) {
> > + if (i)
> > + osf_stream_discard(stream, i);
> > + return i;
> > + }
> > + }
> > +
> > + stream->len = 0;
> > + return old_len;
> > +}
>
> ...
>
> I stop here, because it's obvious that you neglected and ignored my previous
> reviews. No explanation given, nothing. This is not how you should interact
> with the community.
>
> Come again when each of the given comment will be either addressed or argued.
>
Likewise. Please make sure to address every comment either through
changes or through reply to the earlier thread. Perhaps some emails
have gone astray (it happens!). It can be a good idea to take a quick
look at lore.kernel.org to make sure you aren't missing any feedback
on a previous version.
Thanks,
Jonathan
^ permalink raw reply
* Re: [PATCH v4 0/2] arm64: errata: NVIDIA Olympus device store/load ordering
From: Shanker Donthineni @ 2026-06-29 23:08 UTC (permalink / raw)
To: Vladimir Murzin, Catalin Marinas, Will Deacon
Cc: Jason Gunthorpe, linux-arm-kernel, Mark Rutland, linux-kernel,
linux-doc, Vikram Sethi, Jason Sequeira
In-Reply-To: <381fb71c-0a2c-4dec-98a3-56ad88e190c6@arm.com>
Hi Vladimir,
On 6/29/2026 5:45 AM, Vladimir Murzin wrote:
> External email: Use caution opening links or attachments
>
>
> Hi,
>
> On 6/25/26 19:24, Shanker Donthineni wrote:
>> This series works around the NVIDIA Olympus device store/load ordering
>> erratum (T410-OLY-1027): a Device-nGnR* load can be observed by a
>> peripheral before an older, non-overlapping Device-nGnR* store to the
>> same peripheral, breaking the program order that drivers rely on for
>> MMIO and potentially leaving a device in an incorrect state.
>>
>> Patch 1 adds the workaround. It promotes the raw MMIO store helpers
>> (__raw_writeb/w/l/q, and therefore writel()/writel_relaxed()) to
>> store-release on affected CPUs, and promotes the trailing DGH of the
>> write-combining __iowrite{32,64}_copy() helpers to dmb osh. Everything is
>> gated on a new ARM64_WORKAROUND_DEVICE_STORE_RELEASE cpucap and patched
>> in only on affected parts, so it is a no-op elsewhere.
>>
>> Patch 2 provides arm64 memset_io()/memcpy_toio(). The generic versions
>> are built on __raw_write*(), so patch 1 would promote every store in a
>> block to a store-release; as each STLR drains the write-combining buffer,
>> block MMIO becomes O(n) store-releases. The arm64 versions emit plain
>> STR in the loop and order the whole block with a single trailing dmb osh,
>> keeping block MMIO at one-barrier cost.
>>
>> Performance: NVIDIA Olympus, write-combining MMIO to a device BAR, single
>> PE pinned; per-call cost in ns. Consecutive writes ping-pong between two
>> buffers so repeated stores are not coalesced. iowrite64/iowrite32 =
>> __iowrite{64,32}_copy().
>>
>> Table 1 - workaround off (CONFIG_NVIDIA_OLYMPUS_1027_ERRATUM=n)
>> +-------+-----------+-----------+-----------+-------------+
>> | size | iowrite64 | iowrite32 | memset_io | memcpy_toio |
>> +-------+-----------+-----------+-----------+-------------+
>> | 8B | 67.9 ns | 67.8 ns | 3.6 ns | 3.6 ns |
>> | 16B | 67.9 ns | 67.8 ns | 4.0 ns | 4.0 ns |
>> | 32B | 67.9 ns | 67.9 ns | 4.6 ns | 4.6 ns |
>> | 64B | 69.1 ns | 69.1 ns | 69.1 ns | 69.0 ns |
>> | 128B | 138.3 ns | 138.3 ns | 138.4 ns | 138.3 ns |
>> | 256B | 276.6 ns | 276.6 ns | 276.6 ns | 276.7 ns |
>> | 512B | 276.6 ns | 276.5 ns | 276.6 ns | 276.6 ns |
>> | 1KB | 276.6 ns | 278.4 ns | 276.6 ns | 276.6 ns |
>> | 2KB | 278.4 ns | 278.4 ns | 275.9 ns | 276.6 ns |
>> | 4KB | 365.7 ns | 365.7 ns | 365.7 ns | 365.7 ns |
>> +-------+-----------+-----------+-----------+-------------+
>> relaxed/no-flush: memset_io()/memcpy_toio() issue plain stores with no
>> trailing dgh() or barrier, unlike __iowrite*_copy() which ends with dgh().
>>
>> Table 2 - workaround on, arm64 memset_io/memcpy_toio (this series)
>> +-------+-----------+-----------+-----------+-------------+
>> | size | iowrite64 | iowrite32 | memset_io | memcpy_toio |
>> +-------+-----------+-----------+-----------+-------------+
>> | 8B | 231.6 ns | 231.6 ns | 232.4 ns | 232.4 ns |
>> | 16B | 231.7 ns | 231.9 ns | 232.7 ns | 232.6 ns |
>> | 32B | 231.9 ns | 232.7 ns | 232.9 ns | 232.9 ns |
>> | 64B | 232.7 ns | 235.0 ns | 233.7 ns | 233.6 ns |
>> | 128B | 233.6 ns | 235.8 ns | 234.4 ns | 234.3 ns |
>> | 256B | 237.7 ns | 276.8 ns | 264.0 ns | 276.7 ns |
>> | 512B | 237.7 ns | 277.1 ns | 238.1 ns | 277.6 ns |
>> | 1KB | 253.7 ns | 279.3 ns | 276.1 ns | 294.1 ns |
>> | 2KB | 295.0 ns | 318.7 ns | 288.5 ns | 308.3 ns |
>> | 4KB | 365.9 ns | 381.4 ns | 365.7 ns | 381.3 ns |
>> +-------+-----------+-----------+-----------+-------------+
>> all four helpers end with a single trailing barrier (dmb osh).
>>
>> Table 3 - workaround on, generic per-store memset_io/memcpy_toio
>> +-------+-----------+-----------+-------------+--------------+
>> | size | iowrite64 | iowrite32 | memset_io | memcpy_toio |
>> +-------+-----------+-----------+-------------+--------------+
>> | 8B | 231.6 ns | 231.6 ns | 229.0 ns | 229.0 ns |
>> | 16B | 231.7 ns | 231.9 ns | 458.4 ns | 458.5 ns |
>> | 32B | 231.9 ns | 232.7 ns | 917.4 ns | 917.5 ns |
>> | 64B | 232.7 ns | 234.8 ns | 1835.4 ns | 1835.5 ns |
>> | 128B | 233.6 ns | 235.8 ns | 3670.9 ns | 3670.8 ns |
>> | 256B | 237.7 ns | 276.7 ns | 7341.6 ns | 7341.6 ns |
>> | 512B | 237.7 ns | 279.4 ns | 14001.4 ns | 14001.3 ns |
>> | 1KB | 253.7 ns | 279.1 ns | 28631.5 ns | 28631.8 ns |
>> | 2KB | 279.4 ns | 317.9 ns | 57276.3 ns | 57275.2 ns |
>> | 4KB | 365.7 ns | 381.5 ns | 114564.4 ns | 114563.6 ns |
>> +-------+-----------+-----------+-------------+--------------+
>> the generic memset_io()/memcpy_toio() build on __raw_write*(), which the
>> workaround promotes to store-release, so every store is individually
>> ordered - hence O(n) in the store count.
>>
>> Tables 2 and 3 show why patch 2 is needed: the generic per-store block
>> writers collapse to O(n) under the workaround (4KB ~314x slower, ~115 us
>> vs ~366 ns), while the arm64 versions stay flat at one-barrier cost.
> That's interesting. With the way the patch set is structured, it
> now looks like:
>
> 1. Fix the erratum, but cause a performance regression.
> 2. Restore the performance regression and (re)apply the erratum
> workaround.
>
> Would it make sense to avoid introducing the performance
> regression in the first place by structuring the patch set
> slightly differently?
>
> 1. (Re)introduce arm64 memset_io()/memcpy_toio().
> 2. Fix the erratum once for all
>
> What do you reckon?
Yes, that ordering makes sense.
I can restructure v5 so that patch 1 introduces the arm64 memset_{to}io()
implementations while preserving the existing behavior. Patch 2 will
then add the complete erratum workaround, including the conditional
trailing DMB for those block-write helpers. This avoids introducing
the intermediate performance regression and keeps each commit
independently usable.
Will and Catalin, could you please share your thoughts on this approach?
-Shanker
^ permalink raw reply
* Re: [PATCH v4 2/2] arm64: io: apply the device store-release workaround once per block write
From: Shanker Donthineni @ 2026-06-29 23:09 UTC (permalink / raw)
To: Vladimir Murzin, Catalin Marinas, Will Deacon
Cc: Jason Gunthorpe, linux-arm-kernel, Mark Rutland, linux-kernel,
linux-doc, Vikram Sethi, Jason Sequeira
In-Reply-To: <97b62a6f-a514-46bb-9ee8-81f563220f6a@arm.com>
Hi Vladimir,
On 6/29/2026 5:48 AM, Vladimir Murzin wrote:
> External email: Use caution opening links or attachments
>
>
> Hi,
>
> On 6/25/26 19:24, Shanker Donthineni wrote:
>> The generic memset_io()/memcpy_toio() are built on __raw_write*(), so on
>> parts with the NVIDIA Olympus device store/load ordering erratum the
>> ARM64_WORKAROUND_DEVICE_STORE_RELEASE workaround promotes every store in
>> the block to a store-release. Each stlr* carries a barrier cost, so block
>> MMIO becomes O(n) store-releases, making a block copy many times slower
>> than a single ordered burst and growing with the transfer size.
>>
>> Provide arm64 memset_io()/memcpy_toio() that emit plain str* in the loop
>> and order the whole block against subsequent loads with a single
>> trailing dmb osh on affected CPUs (a no-op elsewhere, preserving the
>> relaxed contract of these helpers). This keeps block MMIO writes at
>> one-barrier cost rather than scaling with the transfer size.
>>
>> Performance (NVIDIA Olympus, write-combining MMIO to a device BAR, single
>> PE pinned; per-call cost in ns; consecutive writes ping-pong between two
>> buffers so repeated stores are not coalesced; iowrite64/iowrite32 =
>> __iowrite{64,32}_copy()):
>>
>> Table 1 - arm64 memset_io/memcpy_toio (this patch)
>> +-------+-----------+-----------+-----------+-------------+
>> | size | iowrite64 | iowrite32 | memset_io | memcpy_toio |
>> +-------+-----------+-----------+-----------+-------------+
>> | 8B | 231.6 ns | 231.6 ns | 232.4 ns | 232.4 ns |
>> | 16B | 231.7 ns | 231.9 ns | 232.7 ns | 232.6 ns |
>> | 32B | 231.9 ns | 232.7 ns | 232.9 ns | 232.9 ns |
>> | 64B | 232.7 ns | 235.0 ns | 233.7 ns | 233.6 ns |
>> | 128B | 233.6 ns | 235.8 ns | 234.4 ns | 234.3 ns |
>> | 256B | 237.7 ns | 276.8 ns | 264.0 ns | 276.7 ns |
>> | 512B | 237.7 ns | 277.1 ns | 238.1 ns | 277.6 ns |
>> | 1KB | 253.7 ns | 279.3 ns | 276.1 ns | 294.1 ns |
>> | 2KB | 295.0 ns | 318.7 ns | 288.5 ns | 308.3 ns |
>> | 4KB | 365.9 ns | 381.4 ns | 365.7 ns | 381.3 ns |
>> +-------+-----------+-----------+-----------+-------------+
>> all four helpers end with a single trailing barrier (dmb osh).
>>
>> Table 2 - generic per-store memset_io/memcpy_toio
>> +-------+-----------+-----------+-------------+--------------+
>> | size | iowrite64 | iowrite32 | memset_io | memcpy_toio |
>> +-------+-----------+-----------+-------------+--------------+
>> | 8B | 231.6 ns | 231.6 ns | 229.0 ns | 229.0 ns |
>> | 16B | 231.7 ns | 231.9 ns | 458.4 ns | 458.5 ns |
>> | 32B | 231.9 ns | 232.7 ns | 917.4 ns | 917.5 ns |
>> | 64B | 232.7 ns | 234.8 ns | 1835.4 ns | 1835.5 ns |
>> | 128B | 233.6 ns | 235.8 ns | 3670.9 ns | 3670.8 ns |
>> | 256B | 237.7 ns | 276.7 ns | 7341.6 ns | 7341.6 ns |
>> | 512B | 237.7 ns | 279.4 ns | 14001.4 ns | 14001.3 ns |
>> | 1KB | 253.7 ns | 279.1 ns | 28631.5 ns | 28631.8 ns |
>> | 2KB | 279.4 ns | 317.9 ns | 57276.3 ns | 57275.2 ns |
>> | 4KB | 365.7 ns | 381.5 ns | 114564.4 ns | 114563.6 ns |
>> +-------+-----------+-----------+-------------+--------------+
>> the generic memset_io()/memcpy_toio() build on __raw_write*(), which the
>> workaround promotes to store-release, so every store is individually
>> ordered - hence O(n) in the store count.
>>
>> The arm64 versions stay flat at one-barrier cost while the generic
>> per-store writers collapse to O(n): at 4KB ~314x slower (~115 us vs
>> ~366 ns).
>>
>> Signed-off-by: Shanker Donthineni <sdonthineni@nvidia.com>
>> ---
>> arch/arm64/include/asm/io.h | 5 +++
>> arch/arm64/kernel/io.c | 82 +++++++++++++++++++++++++++++++++++++
>> 2 files changed, 87 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/io.h b/arch/arm64/include/asm/io.h
>> index 69e0fa004d31..649503f347bc 100644
>> --- a/arch/arm64/include/asm/io.h
>> +++ b/arch/arm64/include/asm/io.h
>> @@ -266,6 +266,11 @@ __iowrite64_copy(void __iomem *to, const void *from, size_t count)
>> }
>> #define __iowrite64_copy __iowrite64_copy
>>
>> +void memset_io(volatile void __iomem *dst, int c, size_t count);
>> +#define memset_io memset_io
>> +void memcpy_toio(volatile void __iomem *dst, const void *src, size_t count);
>> +#define memcpy_toio memcpy_toio
>> +
>> /*
>> * I/O memory mapping functions.
>> */
>> diff --git a/arch/arm64/kernel/io.c b/arch/arm64/kernel/io.c
>> index fe86ada23c7d..b5fd9ee6d9eb 100644
>> --- a/arch/arm64/kernel/io.c
>> +++ b/arch/arm64/kernel/io.c
>> @@ -5,9 +5,91 @@
>> * Copyright (C) 2012 ARM Ltd.
>> */
>>
>> +#include <linux/align.h>
>> #include <linux/export.h>
>> #include <linux/types.h>
>> #include <linux/io.h>
>> +#include <linux/unaligned.h>
>> +
>> +#include <asm/alternative.h>
>> +
>> +/*
>> + * ARM64_WORKAROUND_DEVICE_STORE_RELEASE promotes every raw MMIO store
>> + * (__raw_write*()) to a store-release on affected CPUs. The generic
>> + * memset_io()/memcpy_toio() are built on those helpers, so the workaround would
>> + * emit one store-release per element and turn a block write into O(n) ordered
>> + * stores - far more costly than the single barrier a block actually needs.
>> + *
>> + * Provide arm64 versions that emit plain STR in the loop and order the whole
>> + * block against subsequent loads with one trailing DMB OSH, patched in only on
>> + * affected CPUs (a no-op elsewhere, so the relaxed contract of these helpers is
>> + * preserved).
>> + *
>> + * This capability is currently enabled only for the NVIDIA Olympus device
>> + * store/load ordering erratum, where a Device-nGnR* load may be observed before
>> + * an older, non-overlapping Device-nGnR* store to the same peripheral.
>> + */
>> +static __always_inline void iomem_block_store_barrier(void)
>> +{
>> + asm volatile(ALTERNATIVE("nop", "dmb osh",
>> + ARM64_WORKAROUND_DEVICE_STORE_RELEASE)
>> + : : : "memory");
>> +}
>> +
>> +void memset_io(volatile void __iomem *dst, int c, size_t count)
>> +{
>> + u64 qc = (u8)c;
>> +
>> + qc *= ~0ULL / 0xff;
>> +
>> + while (count && !IS_ALIGNED((__force unsigned long)dst, sizeof(u64))) {
>> + asm volatile("strb %w0, [%1]" : : "rZ"((u8)c), "r"(dst) : "memory");
>> + dst++;
>> + count--;
>> + }
>> + while (count >= sizeof(u64)) {
>> + asm volatile("str %x0, [%1]" : : "rZ"(qc), "r"(dst) : "memory");
>> + dst += sizeof(u64);
>> + count -= sizeof(u64);
>> + }
>> + while (count) {
>> + asm volatile("strb %w0, [%1]" : : "rZ"((u8)c), "r"(dst) : "memory");
>> + dst++;
>> + count--;
>> + }
>> +
>> + iomem_block_store_barrier();
>> +}
>> +EXPORT_SYMBOL(memset_io);
>> +
>> +void memcpy_toio(volatile void __iomem *dst, const void *src, size_t count)
>> +{
>> + while (count && !IS_ALIGNED((__force unsigned long)dst, sizeof(u64))) {
>> + asm volatile("strb %w0, [%1]"
>> + : : "rZ"(*(const u8 *)src), "r"(dst) : "memory");
>> + src++;
>> + dst++;
>> + count--;
>> + }
>> + while (count >= sizeof(u64)) {
>> + asm volatile("str %x0, [%1]"
>> + : : "rZ"(get_unaligned((const u64 *)src)), "r"(dst)
> Why do we need get_unaligned() here? I understand this came from
> the generic implementation, where it needs to handle architectures
> that do not support unaligned accesses. But IIUC this is not an
> issue for arm64, and there was no special handling in memcpy_toio()
> before 0110feaaf6d0 ("arm64: Use new fallback IO memcpy/memset").
> Am I missing something?
Thanks for the review.
I used get_unaligned() because I was trying to keep the arm64 implementation
as close as possible to the generic memcpy_toio() implementation in
lib/iomem_copy.c. However, you are right that before commit 0110feaaf6d0
(“arm64: Use new fallback IO memcpy/memset”), the arm64 implementation
used a direct u64 load and did not explicitly handle source alignment. I
can restore the previous arm64 form in v5 if that is preferred.
>> + : "memory");
>> + src += sizeof(u64);
>> + dst += sizeof(u64);
>> + count -= sizeof(u64);
>> + }
>> + while (count) {
>> + asm volatile("strb %w0, [%1]"
>> + : : "rZ"(*(const u8 *)src), "r"(dst) : "memory");
>> + src++;
>> + dst++;
>> + count--;
>> + }
>> +
>> + iomem_block_store_barrier();
> It is perhaps a matter of taste, but having the inline assembly
> here (and in memset_io()) might make the code clearer. To a
> casual reader, it would be obvious that the barrier is not
> guaranteed and is only applicable to ARM64_WORKAROUND_DEVICE_STORE_RELEASE,
> without having to jump back and forth through the code.
>
> Obliviously maintainers might have different preference ;)
Regarding the barrier, iomem_block_store_barrier() is declared
static __always_inline, so it does not add a function call. The nop/dmb
osh alternative is emitted directly in each caller. I used the helper to
avoid duplicating the alternative sequence.
I understand that placing the assembly directly in both functions could
make its conditional nature more obvious. I do not have a strong preference
and am happy to follow Will’s and Catalin’s preference here.
-Shanker
^ permalink raw reply
* Re: [PATCH v8 23/46] KVM: TDX: Make source page optional for KVM_TDX_INIT_MEM_REGION
From: Ackerley Tng @ 2026-06-30 0:00 UTC (permalink / raw)
To: Yan Zhao
Cc: Sean Christopherson, aik, andrew.jones, binbin.wu, brauner,
chao.p.peng, david, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, tabba, willy, wyihan, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <akI9m02jgKAdi4gX@yzhao56-desk.sh.intel.com>
Yan Zhao <yan.y.zhao@intel.com> writes:
> On Fri, Jun 26, 2026 at 08:28:32AM -0700, Ackerley Tng wrote:
>> Yan Zhao <yan.y.zhao@intel.com> writes:
>>
>> > On Thu, Jun 25, 2026 at 05:07:23PM -0700, Ackerley Tng wrote:
>> >> Yan Zhao <yan.y.zhao@intel.com> writes:
>> >>
>> >> > On Wed, Jun 24, 2026 at 04:00:32PM -0700, Ackerley Tng wrote:
>> >> >> Sean Christopherson <seanjc@google.com> writes:
>> >> >>
>> >> >> > On Tue, Jun 23, 2026, Yan Zhao wrote:
>> >> >> >> On Tue, Jun 23, 2026 at 01:16:14PM +0800, Yan Zhao wrote:
>> >> >> >> > On Mon, Jun 22, 2026 at 06:22:45PM -0700, Sean Christopherson wrote:
>> >> >> >> > > On Mon, Jun 22, 2026, Yan Zhao wrote:
>> >> >> >> > > > On Thu, Jun 18, 2026 at 05:32:00PM -0700, Ackerley Tng via B4 Relay wrote:
>> >> >> >> > > > > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>> >> >> >> > > > > index ffe9d0db58c59..56d10333c61a7 100644
>> >> >> >> > > > > --- a/arch/x86/kvm/vmx/tdx.c
>> >> >> >> > > > > +++ b/arch/x86/kvm/vmx/tdx.c
>> >> >> >> > > > > @@ -3198,8 +3198,12 @@ static int tdx_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
>> >> >> >> > > > > if (KVM_BUG_ON(kvm_tdx->page_add_src, kvm))
>> >> >> >> > > > > return -EIO;
>> >> >> >> > > > >
>> >> >> >> > > > > - if (!src_page)
>> >> >> >> > > > > - return -EOPNOTSUPP;
>> >> >> >> > > > > + if (!src_page) {
>> >> >> >> > > > > + if (!gmem_in_place_conversion)
>> >> >> >> > > > When userspace turns on gmem_in_place_conversion while creating guest_memfd
>> >> >> >> > > > without the MMAP flag, the absence of src_page should still be treated as an
>> >> >> >> > > > error.
>> >> >> >> > >
>> >> >> >> > > Why MMAP?
>> >> >> >> > Hmm, I was showing a scenario that in-place conversion couldn't occur.
>> >> >> >> > I didn't mean that with the MMAP flag, mmap() and user write must occur.
>> >> >> >> >
>> >> >> >> > > Shouldn't this be a general "if (!src_page && !up-to-date)"? Just
>> >> >> >> > > because userspace _can_ mmap() the memory doesn't mean userspace _has_ mmap()'d
>> >> >> >> > > and written memory. And when write() lands, MMAP wouldn't be necessary to
>> >> >> >> > > initialize the memory.
>> >> >> >> > Do you mean using up-to-date flag as below?
>> >> >> >
>> >> >> > Yes? I didn't actually look at the implementation details.
>> >> >> >
>> >> >> >> > if (!src_page) {
>> >> >> >> > src_page = pfn_to_page(pfn);
>> >> >> >> > if (!folio_test_uptodate(page_folio(src_page)))
>> >> >> >> > return -EOPNOTSUPP;
>> >> >> >> > }
>> >> >>
>> >> >> Yan is right that with the earlier patch "Zero page while getting pfn",
>> >> >> folio_test_uptodate() here will always return true.
>> >> >>
>> >> >> Actually, this is an alternative fix for the issue Sashiko pointed out
>> >> >> on v7 where userspace can do a populate() (either TDX or SNP) without
>> >> >> first allocating the page, with src_address == NULL, and leak
>> >> >> uninitialized memory into the guest.
>> >> >>
>> >> >> Advantage of using the uptodate check in populate: if the host never
>> >> >> allocates the page, populate doesn't incur zeroing before writing the
>> >> >> page anyway in populate().
>> >> >>
>> >> >> Disadvantage: Both TDX and SNP will have to implement this uptodate
>> >> >> check. guest_memfd can't check centrally because for SNP, for a
>> >> >> PAGE_TYPE_ZERO, !src_page should be allowed with a !uptodate page since
>> >> >> firmware will zero and there's no leakage of uninitialized host memory?
>> >> > Another disadvantage: the uptodate flag is per-folio. What if the folio
>> >> > is only partially initialized by the userspace especially after huge page is
>> >> > supported?
>> >> >
>> >>
>> >> Good point on huge pages!
>> >>
>> >> The uptodate flag on the folio in guest_memfd means "this folio has been
>> >> written to". As of now (before patch at [1]), this happens when
>> >>
>> >> + folio is zeroed on first use by userspace
>> >> + folio is zeroed on first use of the guest
>> >> + folio is populated
>> >>
>> >> When huge pages are supported, the folio can't partially be initialized?
>> >>
>> >> On allocation, if any part is shared, we split the page. The parts are
>> >> separate folios that have their own uptodate flags.
>> >>
>> >> On splitting, if the huge page is uptodate, the split pages will also be
>> >> uptodate. If the huge page is not uptodate, the split pages won't be
>> >> uptodate, but that's ok since they will be marked uptodate on first use.
>> >>
>> >> On merging, the non-uptodate parts have to be zeroed and then marked
>> > If that's true, it would be good.
>> >
>> >> uptodate. Any parts that are in use would have been marked uptodate
>> >> already, so there's no overwriting data that is in use. I'll need to
>> >> think more about when it's safe to zero.
>> >>
>> >> I'm still on the fence between the two options
>> >>
>> >> 1. Using uptodate check in populate to reject src_pages that have never
>> >> been written to or
>> >> 2. Always zero before populate
>> > 2 does not work?
>> > The flow is
>> > 1. mmap gmem_fd, make GFN shared, and write initial content.
>> > 2. convert GFN to private
>> > 3. invoke ioctl to trigger populate.
>> >
>>
>> This flow is correct, is what users of in-place conversion should do.
>>
>> "Always" is the wrong word, I should have said "zero if not uptodate
>> before populate", as in, with patch at [1].
>>
>> By doing the zeroing in __kvm_gmem_get_pfn instead, by the time populate
>> gets the pfn, the page would be zeroed, either because userspace faulted
>> it in, and the zeroing happened in kvm_gmem_fault_user_mapping(), or if
>> userspace never faulted it in, the zeroing would happen because
>> populate() allocated the page.
>
> I see.
>
>> >> but whether the uptodate flag is per-folio or not doesn't affect these
>> >> two options in terms of fixing the leak of uninitialized host memory,
>> >> right?
>> > yes, provided "On merging, the non-uptodate parts have to be zeroed and then
>> > marked uptodate".
>> >
>>
>> Thank you so much for bringing this up, I hadn't considered this
>> before. I'll do that when I get to guest_memfd hugepage restructuring.
>>
>> >> >
>> >> >> >> Another concern with this fix is that:
>> >> >> >> commit "KVM: guest_memfd: Zero page while getting pfn" [1] always marks the
>> >> >> >> folio uptodate before reaching post_populate().
>> >> >> >>
>> >> >> >> [1] https://lore.kernel.org/all/20260618-gmem-inplace-conversion-v8-21-9d2959357853@google.com/
>> >> >> >>
>> >> >> >> > One concern is that TDX now does not much care about the up-to-date flag since
>> >> >> >> > TDX doesn't rely on the flag to clear pages on conversions.
>> >> >> >> > I'm not sure if the flag can be reliably checked in this case. e.g.,
>> >> >> >> > now the whole folio is marked up-to-date even if only part of it is faulted by
>> >> >> >> > user access.
>> >> >> >> > Ensuring that the up-to-date flag works correctly with huge page support seems
>> >> >> >> > to have more effort than introducing a dedicated flag for TDX.
>> >> >> >> >
>> >> >> >> > > > Additionally, to properly enable in-place copying for the TDX initial memory
>> >> >> >> > > > region, userspace must not only specify source_addr to NULL, but also follow
>> >> >> >> > > > a specific sequence (where steps 1/2/3/7 are required only for in-place copy):
>> >> >> >> > > > 1. create guest_memfd with MMAP flag
>> >> >> >> > > > 2. mmap the guest_memfd.
>> >> >> >> > > > 3. convert the initial memory range to shared.
>> >> >> >> > > > 4. copy initial content to the source page.
>> >> >> >> > > > 5. convert the initial memory range to private
>> >> >> >> > > > 6. invoke ioctl KVM_TDX_INIT_MEM_REGION.
>> >> >> >> > > > 7. do not unmap the source backend.
>> >> >> >> > > >
>> >> >> >> > > > So, would it be reasonable to introduce a dedicated flag that allows userspace
>> >> >> >> > > > to explicitly opt into the in-place copy functionality? e.g.,
>> >> >> >> > >
>> >> >> >> > > Why? It's userspace's responsibility to get the above right. If userspace fails
>> >> >> >> > > to provide a src_page when it doesn't want in-place copy, that's a userspace bug.
>> >> >>
>> >> >> Yan, is your concern that userspace forgot to update the code and
>> >> >> forgets to provide a src_page, and if we keep the "Zero page while
>> >> > Yes. Previously, it would be rejected after GUP fails.
>> >> >
>> >>
>> >> I see, didn't realize previously it would be rejected because GUP
>> >> fails. GUP failed because it wasn't faulted into the host?
>> > GUP fails if 0 is not a valid user address.
>> > But GUP would not fail if 0 is a valid address. e.g., in below scenario:
>> >
>> > #include <sys/mman.h>
>> > #include <stdio.h>
>> > int main(void)
>> > {
>> > void *p=mmap((void*)0,4096,PROT_READ|PROT_WRITE, MAP_FIXED|MAP_PRIVATE|MAP_ANONYMOUS,-1,0);
>> > if (p==MAP_FAILED) {
>> > perror("mmap");
>> > return 1;
>> > }
>> > *(char*)0='Y';
>> > printf("addr0=%p val=%c\n",p,*(char*)0);
>> > return 0;
>> > }
>> >
>> >
>> >> That's kind of orthogonal, I don't think GUP fail leading to rejecting
>> >> populate was meant to help userspace catch these issues. GUP would also
>> >> fail if the user did mmap(), write to it, unmap using
>> >> madvise(MADV_DONTNEED), then forget and pass 0 as src_address.
>> > The original uAPI did not explicitly define 0 as an invalid uaddr. Whether 0 was
>> > rejected depended on whether the user mmap()'d address 0. If 0 was a valid
>> > mapping, populate() could proceed.
>> >
>> > commit 2a62345b3052 ("KVM: guest_memfd: GUP source pages prior to populating
>> > guest memory") changed the behavior though. It would return -EOPNOTSUPP for a 0
>> > uaddr.
>> >
>>
>> I see, I only looked at this after commit 2a62345b3052.
>>
>> > But if a user configures 0 uaddr as valid, writes to it, and then passes 0 as
>> > source_addr(not from gmem), I'm not sure if it's good for the kernel to silently
>> > treat 0 uaddr as an identifier for in-place copy from the private PFN in gmem.
>> >
>>
>> I'd say the original uAPI perhaps just didn't document 0 as an
>> unsupported uaddr. Given that commit 2a62345b3052 already merged, uAPI
>> was perhaps accidentally changed and no customer complained, I think we
>> can move forward with 0 as an invalid src_address? I wouldn't think
>> anyone relies on 0 intentionally being a valid address.
>>
>> I could document that, if it helps?
> What about just documenting that 0 is an unsupported uaddr which will be
> re-purposed as an indicator to use the target pfn as the source, regardless of
> whether gmem_in_place_conversion is true? i.e.,
>
> if (!src_page)
> src_page = pfn_to_page(pfn);
>
> I don't get why the two scenarios should be treated differently:
> 1. gmem_in_place_conversion==true, shared memory is not from gmem
> 2. gmem_in_place_conversion==false, shared memory is not from gmem
>
> In both case, a 0 uaddr could be mapped to a valid page not from gmem.
This is true, but this check isn't about whether the page is from gmem.
> So why not update the uAPI to handle both cases consistently? :)
>
Wait, but before this series, if region.src_address = 0, src_page = NULL
and that's not supported so it returns -EOPNOTSUPP.
If that's dropped, then suddenly if region.src_address = 0 and
!gmem_in_place_conversion, tdx_gmem_post_populate() will now load the
memory (zeroed) after [1] into the guest? I don't think we want to
change that behavior.
I could document that 0 is an unsupported uaddr only for TDX, and only
when gmem_in_place_conversion = false.
Since it is unsupported only when gmem_in_place_conversion = false, the
check two lines marked with <<==== can't go away?
if (!src_page) {
if (!gmem_in_place_conversion) <<====
return -EOPNOTSUPP; <<====
src_page = pfn_to_page(pfn);
}
Also, for SNP, src_address == 0 is permitted (and desired, I believe, to
avoid a pointless kernel memcpy) if the type of population is
KVM_SEV_SNP_PAGE_TYPE_ZERO.
>> >> >> getting pfn" patch, ends up with the guest silently having a zero page?
>> >> >> I think that would be found quite early in userspace VMM testing...
>>
>> [...snip...]
>>
^ permalink raw reply
* Re: [PATCH v2] docs/mm: Fix braces
From: SJ Park @ 2026-06-30 0:06 UTC (permalink / raw)
To: Manuel Ebner
Cc: SJ Park, Jason Gunthorpe, Leon Romanovsky, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Jonathan Corbet, Shuah Khan, Shakeel Butt, Randy Dunlap, linux-mm,
linux-doc, linux-kernel
In-Reply-To: <20260629161156.90213-2-manuelebner@mailbox.org>
On Mon, 29 Jun 2026 18:11:56 +0200 Manuel Ebner <manuelebner@mailbox.org> wrote:
> Correct typos in mm documentation by balancing parentheses.
Good eyes!
>
> Signed-off-by: Manuel Ebner <manuelebner@mailbox.org>
> Acked-by: Randy Dunlap <rdunlap@infradead.org>
> Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>
Reviewed-by: SeongJae Park <sj@kernel.org>
Thanks,
SJ
[...]
^ permalink raw reply
* Re: [PATCH v2] docs/mm: Fix braces
From: SJ Park @ 2026-06-30 0:13 UTC (permalink / raw)
To: SJ Park
Cc: Manuel Ebner, Jason Gunthorpe, Leon Romanovsky, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Jonathan Corbet, Shuah Khan, Shakeel Butt, Randy Dunlap, linux-mm,
linux-doc, linux-kernel
In-Reply-To: <20260630000620.138992-1-sj@kernel.org>
On Mon, 29 Jun 2026 17:06:19 -0700 SJ Park <sj@kernel.org> wrote:
> On Mon, 29 Jun 2026 18:11:56 +0200 Manuel Ebner <manuelebner@mailbox.org> wrote:
>
> > Correct typos in mm documentation by balancing parentheses.
>
> Good eyes!
>
> >
> > Signed-off-by: Manuel Ebner <manuelebner@mailbox.org>
> > Acked-by: Randy Dunlap <rdunlap@infradead.org>
> > Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>
>
> Reviewed-by: SeongJae Park <sj@kernel.org>
I mean,
Reviewed-by: SJ Park <sj@kernel.org>
Sorry for the noise.
Thanks,
SJ
[...]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox