* [PATCH v4 01/10] x86/acpi: Add a helper functions to setup and access the wakeup mailbox
From: Ricardo Neri @ 2025-06-04 0:15 UTC (permalink / raw)
To: x86, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley, Rafael J. Wysocki
Cc: Saurabh Sengar, Chris Oo, Kirill A. Shutemov, linux-hyperv,
devicetree, linux-acpi, linux-kernel, Ravi V. Shankar,
Ricardo Neri, Ricardo Neri
In-Reply-To: <20250603-rneri-wakeup-mailbox-v4-0-d533272b7232@linux.intel.com>
In preparation to move the functionality to wake secondary CPUs up out of
the ACPI code, add two helper functions.
The function acpi_setup_mp_wakeup_mailbox() stores the physical address of
the mailbox and updates the wakeup_secondary_cpu_64() APIC callback.
There is a slight change in behavior: now the APIC callback is updated
before configuring CPU hotplug offline behavior. This is fine as the APIC
callback continues to be updated unconditionally, regardless of the
restriction on CPU offlining.
The function acpi_madt_multiproc_wakeup_mailbox() returns a pointer to the
mailbox. Use this helper function only in the portions of the code for
which the variable acpi_mp_wake_mailbox will be out of scope once it is
relocated out of the ACPI directory.
The wakeup mailbox is only supported for CONFIG_X86_64 and needed only with
CONFIG_SMP=y.
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v3:
- Squashed the two first patches of the series into one, both introduce
helper functions. (Rafael)
- Renamed setup_mp_wakeup_mailbox() as acpi_setup_mp_wakeup_mailbox().
(Rafael)
- Dropped the function prototype for !CONFIG_X86_64. (Rafael)
Changes since v2:
- Introduced this patch.
Changes since v1:
- N/A
---
arch/x86/include/asm/smp.h | 3 +++
arch/x86/kernel/acpi/madt_wakeup.c | 20 +++++++++++++++-----
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 0c1c68039d6f..77dce560a70a 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -146,6 +146,9 @@ static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
return per_cpu(cpu_l2c_shared_map, cpu);
}
+void acpi_setup_mp_wakeup_mailbox(u64 addr);
+struct acpi_madt_multiproc_wakeup_mailbox *acpi_get_mp_wakeup_mailbox(void);
+
#else /* !CONFIG_SMP */
#define wbinvd_on_cpu(cpu) wbinvd()
static inline int wbinvd_on_all_cpus(void)
diff --git a/arch/x86/kernel/acpi/madt_wakeup.c b/arch/x86/kernel/acpi/madt_wakeup.c
index f36f28405dcc..4033c804307a 100644
--- a/arch/x86/kernel/acpi/madt_wakeup.c
+++ b/arch/x86/kernel/acpi/madt_wakeup.c
@@ -37,6 +37,7 @@ static void acpi_mp_play_dead(void)
static void acpi_mp_cpu_die(unsigned int cpu)
{
+ struct acpi_madt_multiproc_wakeup_mailbox *mailbox = acpi_get_mp_wakeup_mailbox();
u32 apicid = per_cpu(x86_cpu_to_apicid, cpu);
unsigned long timeout;
@@ -46,13 +47,13 @@ static void acpi_mp_cpu_die(unsigned int cpu)
*
* BIOS has to clear 'command' field of the mailbox.
*/
- acpi_mp_wake_mailbox->apic_id = apicid;
- smp_store_release(&acpi_mp_wake_mailbox->command,
+ mailbox->apic_id = apicid;
+ smp_store_release(&mailbox->command,
ACPI_MP_WAKE_COMMAND_TEST);
/* Don't wait longer than a second. */
timeout = USEC_PER_SEC;
- while (READ_ONCE(acpi_mp_wake_mailbox->command) && --timeout)
+ while (READ_ONCE(mailbox->command) && --timeout)
udelay(1);
if (!timeout)
@@ -227,7 +228,7 @@ int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
acpi_table_print_madt_entry(&header->common);
- acpi_mp_wake_mailbox_paddr = mp_wake->mailbox_address;
+ acpi_setup_mp_wakeup_mailbox(mp_wake->mailbox_address);
if (mp_wake->version >= ACPI_MADT_MP_WAKEUP_VERSION_V1 &&
mp_wake->header.length >= ACPI_MADT_MP_WAKEUP_SIZE_V1) {
@@ -243,7 +244,16 @@ int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
acpi_mp_disable_offlining(mp_wake);
}
+ return 0;
+}
+
+void __init acpi_setup_mp_wakeup_mailbox(u64 mailbox_paddr)
+{
+ acpi_mp_wake_mailbox_paddr = mailbox_paddr;
apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu);
+}
- return 0;
+struct acpi_madt_multiproc_wakeup_mailbox *acpi_get_mp_wakeup_mailbox(void)
+{
+ return acpi_mp_wake_mailbox;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v4 00/10] x86/hyperv/hv_vtl: Use a wakeup mailbox to boot secondary CPUs
From: Ricardo Neri @ 2025-06-04 0:15 UTC (permalink / raw)
To: x86, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley, Rafael J. Wysocki
Cc: Saurabh Sengar, Chris Oo, Kirill A. Shutemov, linux-hyperv,
devicetree, linux-acpi, linux-kernel, Ravi V. Shankar,
Ricardo Neri, Ricardo Neri, Yunhong Jiang, Thomas Gleixner
Hi,
Here is a new version of this series. Many thanks to Rob, Rafael,
Krzysztof, and Michael for your feedback. Previous versions can be found in
1], [2], and [3].
The biggest changes in this version are in the DeviceTree bindings and
their relationship with ACPI as well as relocating the ACPI wakeup code to
a new common file that both DeviceTree- and ACPI-based system can use. See
the changelog for details.
If the DeviceTree bindings look good, then the patches should be ready for
review by the x86, ACPI, and Hyper-V maintainers.
Thanks in advance for your feedback!
...
This patchset adds functionality to use a wakeup mailbox to boot secondary
CPUs in Hyper-V VTL level 2 TDX guests with virtual firmware that describes
hardware using a DeviceTree graph. Although this is the target use case,
the use of the mailbox depends solely on it being enumerated in the
DeviceTree.
On x86 platforms, secondary CPUs are typically booted using INIT assert,
de-assert followed by Start-Up IPI messages. Virtual machines can also make
hypercalls to bring up secondary CPUs to a desired execution state. These
two mechanisms require support from the hypervisor. Confidential computing
VMs in a TDX environment cannot use this mechanism because the hypervisor
is considered an untrusted entity.
Linux already supports the ACPI Multiprocessor Wakeup Structure in which
the guest platform firmware boots the secondary CPUs and transfers control
to the kernel using a mailbox. This mechanism does not need involvement
of the VMM. It can be used in a Hyper-V VTL level 2 TDX guest.
Currently, this mechanism can only be used on x86 platforms with firmware
that supports ACPI. There are platforms that use DeviceTree (e.g., OpenHCL
[4]) instead of ACPI to describe the hardware.
Provided that the wakeup mailbox enumerated in a DeviceTree-based platform
firmware is implemented as described in the ACPI specification, the kernel
can used common code for both DeviceTree and ACPI systems. The DeviceTree
firmware does not need to use any ACPI table to publish the mailbox.
This patcheset is structured as follows:
* Relocate portions of the code handling the ACPI Multiprocessor Wakeup
Structure code to a common location. (patches 1, 2)
* Define DeviceTree bindings to enumerate a mailbox as described in
the ACPI specification. (patch 3)
* Find and setup the wakeup mailbox if found in the DeviceTree graph.
(patch 4)
* Prepare Hyper-V VTL2 TDX guests to use the Wakeup Mailbox to boot
secondary CPUs when available. (patches 5-10)
I have tested this patchset on a Hyper-V host with VTL2 OpenHCL, QEMU, and
physical hardware.
Thanks and BR,
Ricardo
Changes since v3:
- Added Reviewed-by: tags from Michael Kelley. Thanks!
- Relocated the common wakeup code from acpi/madt_wakeup.c to a new
smpwakeup.c to be used in DeviceTree- and ACPI-based systems.
- Dropped the x86 CPU bindings as they are not a good fit to document
firmware features.
- Dropped the code that parsed and validated of the `enable-method`
property for cpu@N nodes in x86. Instead, unconditionally parse and use
the wakeup mailbox when found.
- Updated the wakeup mailbox schema to avoid redefing the structure and
operation of the mailbox. Instead, refer to the ACPI specification.
Also clarified that the enumeration of the mailbox is done separately.
- Prefixed helper functions of wakeup code with acpi_.
Changes since v2:
- Only move out of the acpi directory acpi_wakeup_cpu() and its
accessory variables. Use helper functions to access the mailbox as
needed. This also fixed the warnings about unused code with CONFIG_
ACPI=n that Michael reported.
- Major rework of the DeviceTree bindings and schema. Now there is a
reserved-memory binding for the mailbox as well as a new x86 CPU
bindings. Both have `compatible` properties.
- Rework of the code parsing the DeviceTree bindings for the mailbox.
Now configuring the mailbox depends solely on its enumeration in the
DeviceTree and not on Hyper-V VTL2 TDX guest.
- Do not make reserving the first 1MB of memory optional. It is not
needed and may introduce bugs.
- Prepare Hyper-V VTL2 guests to unconditionally use the mailbox in TDX
environments. If the mailbox is not available, booting secondary CPUs
will fail gracefully.
Changes since v1:
- Fix the cover letter's summary phrase.
- Fix the DT binding document to pass validation.
- Change the DT binding document to be ACPI independent.
- Move ACPI-only functions into the #ifdef CONFIG_ACPI.
- Change dtb_parse_mp_wake() to return mailbox physical address.
- Rework the hv_is_private_mmio_tdx().
- Remove unrelated real mode change from the patch that marks mailbox
page private.
- Check hv_isolation_type_tdx() instead of wakeup_mailbox_addr in
hv_vtl_init_platform() because wakeup_mailbox_addr is not parsed yet.
- Add memory range support to reserve_real_mode.
- Remove realmode_reserve callback and use the memory range.
- Move setting the real_mode_header to hv_vtl_init_platform.
- Update comments and commit messages.
- Minor style changes.
[1]. https://lore.kernel.org/r/20240806221237.1634126-1-yunhong.jiang@linux.intel.com
[2]. https://lore.kernel.org/r/20240823232327.2408869-1-yunhong.jiang@linux.intel.com
[3]. https://lore.kernel.org/r/20250503191515.24041-1-ricardo.neri-calderon@linux.intel.com
[4]. https://openvmm.dev/guide/user_guide/openhcl.html
--
2.43.0
---
Ricardo Neri (6):
x86/acpi: Add a helper functions to setup and access the wakeup mailbox
x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
x86/dt: Parse the Wakeup Mailbox for Intel processors
x86/smpwakeup: Add a helper get the address of the wakeup mailbox
x86/hyperv/vtl: Use the wakeup mailbox to boot secondary CPUs
Yunhong Jiang (4):
x86/hyperv/vtl: Set real_mode_header in hv_vtl_init_platform()
x86/realmode: Make the location of the trampoline configurable
x86/hyperv/vtl: Setup the 64-bit trampoline for TDX guests
x86/hyperv/vtl: Mark the wakeup mailbox page as private
.../reserved-memory/intel,wakeup-mailbox.yaml | 48 ++++++++++++
arch/x86/Kconfig | 7 ++
arch/x86/hyperv/hv_vtl.c | 35 ++++++++-
arch/x86/include/asm/smp.h | 4 +
arch/x86/include/asm/x86_init.h | 3 +
arch/x86/kernel/Makefile | 1 +
arch/x86/kernel/acpi/madt_wakeup.c | 76 ++-----------------
arch/x86/kernel/devicetree.c | 47 ++++++++++++
arch/x86/kernel/smpwakeup.c | 88 ++++++++++++++++++++++
arch/x86/kernel/x86_init.c | 3 +
arch/x86/realmode/init.c | 7 +-
11 files changed, 240 insertions(+), 79 deletions(-)
---
base-commit: 8858e8099446963ee6a0fb9f00f361dda52f04d5
change-id: 20250602-rneri-wakeup-mailbox-328efe72803f
Best regards,
--
Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
^ permalink raw reply
* [PATCH v3 1/1] hv_fcopy_uio_daemon: Fix file copy failure between Windows host and Linux guest
From: yasuenag @ 2025-06-03 23:43 UTC (permalink / raw)
To: eahariha
Cc: kys, haiyangz, wei.liu, decui, linux-hyperv, ssengar,
Yasumasa Suenaga
In-Reply-To: <20250603234300.1997-1-yasuenag@gmail.com>
From: Yasumasa Suenaga <yasuenag@gmail.com>
Handle file copy request from the host (e.g. Copy-VMFile commandlet)
correctly.
Store file path and name as __u16 arrays in struct hv_start_fcopy.
Convert directly to UTF-8 string without casting to wchar_t* in fcopyd.
Fix string conversion failure caused by wchar_t size difference between
Linux (32bit) and Windows (16bit). Convert each character to char
if the value is less than 0x80 instead of using wcstombs() call.
Add new check to snprintf() call for target path creation to handle
length differences between PATH_MAX (Linux) and W_MAX_PATH (Windows).
Signed-off-by: Yasumasa Suenaga <yasuenag@gmail.com>
---
tools/hv/hv_fcopy_uio_daemon.c | 37 ++++++++++++++--------------------
1 file changed, 15 insertions(+), 22 deletions(-)
diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
index 0198321d1..86702f39e 100644
--- a/tools/hv/hv_fcopy_uio_daemon.c
+++ b/tools/hv/hv_fcopy_uio_daemon.c
@@ -62,8 +62,11 @@ static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
filesize = 0;
p = path_name;
- snprintf(target_fname, sizeof(target_fname), "%s/%s",
- path_name, file_name);
+ if (snprintf(target_fname, sizeof(target_fname), "%s/%s",
+ path_name, file_name) >= sizeof(target_fname)) {
+ /* target file name is too long */
+ goto done;
+ }
/*
* Check to see if the path is already in place; if not,
@@ -273,6 +276,8 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
while (len < dest_size) {
if (src[len] < 0x80)
dest[len++] = (char)(*src++);
+ else if (src[len] == '0')
+ break;
else
dest[len++] = 'X';
}
@@ -282,27 +287,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
{
- setlocale(LC_ALL, "en_US.utf8");
- size_t file_size, path_size;
- char *file_name, *path_name;
- char *in_file_name = (char *)smsg_in->file_name;
- char *in_path_name = (char *)smsg_in->path_name;
-
- file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
- path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
-
- file_name = (char *)malloc(file_size * sizeof(char));
- path_name = (char *)malloc(path_size * sizeof(char));
-
- if (!file_name || !path_name) {
- free(file_name);
- free(path_name);
- syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
- return HV_E_FAIL;
- }
+ /*
+ * file_name and path_name should have same length with appropriate
+ * member of hv_start_fcopy.
+ */
+ char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
- wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
- wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
+ setlocale(LC_ALL, "en_US.utf8");
+ wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
+ wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
return hv_fcopy_create_file(file_name, path_name, smsg_in->copy_flags);
}
--
2.49.0
^ permalink raw reply related
* [PATCH v3 0/1] hv_fcopy_uio_daemon: Fix file copy failure between Windows host and Linux guest
From: yasuenag @ 2025-06-03 23:42 UTC (permalink / raw)
To: eahariha
Cc: kys, haiyangz, wei.liu, decui, linux-hyperv, ssengar,
Yasumasa Suenaga
In-Reply-To: <e174e3b0-6b62-4996-9854-39c84e10a317@linux.microsoft.com>
From: Yasumasa Suenaga <yasuenag@gmail.com>
Thanks a lot for your advice!
I fixed cosmetic problems and checked it with checkpatch.pl .
And also I fixed subject and description in the commit
as the document pointed.
I hope this version would go well...
Best regards,
Yasumasa
Yasumasa Suenaga (1):
hv_fcopy_uio_daemon: Fix file copy failure between Windows host and
Linux guest
tools/hv/hv_fcopy_uio_daemon.c | 37 ++++++++++++++--------------------
1 file changed, 15 insertions(+), 22 deletions(-)
--
2.49.0
^ permalink raw reply
* Re: [PATCH v2 0/1] Path string from the host should not be treated
From: Easwar Hariharan @ 2025-06-03 21:26 UTC (permalink / raw)
To: yasuenag; +Cc: kys, haiyangz, wei.liu, decui, eahariha, linux-hyperv, ssengar
In-Reply-To: <20250602235612.1542-1-yasuenag@gmail.com>
On 6/2/2025 4:56 PM, yasuenag@gmail.com wrote:
> From: Yasumasa Suenaga <yasuenag@gmail.com>
>
> Hi,
>
> Easwar, thanks a lot for your comment! I fixed where you pointed.
> Let me know if something wrong in this patch - this is my first
> contribution to Linux kernel...
>
> Yasumasa Suenaga (1):
> Path string from the host should not be treated as wchar_t
>
> tools/hv/hv_fcopy_uio_daemon.c | 36 +++++++++++++---------------------
> 1 file changed, 14 insertions(+), 22 deletions(-)
>
Please run scripts/checkpatch.pl --strict on the patch and fix the reported
issues.
As I mentioned previously, https://www.kernel.org/doc/html/latest/process/submitting-patches.html
is a good reference for the expected format of the patch. Specifically:
"Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead
of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy to do frotz",
as if you are giving orders to the codebase to change its behaviour."
The same also applies to the subject line.
Thanks,
Easwar (he/him)
P.S: It's good etiquette to explicitly CC folks who have provided comments
on the subsequent versions
^ permalink raw reply
* Subject: [PATCH v2] vmbus: retrieve connection-id from DeviceTree
From: Hardik Garg @ 2025-06-03 21:01 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, ssengar
Cc: romank, linux-hyperv, linux-kernel, stable, apais
The connection-id determines which hypervisor communication channel the
guest should use to talk to the VMBus host. This patch adds support to
read this value from the DeviceTree where it exists as a property under
the vmbus node with the compatible ID "microsoft,message-connection-id".
The property name follows the format <vendor>,<field> where
"vendor": "microsoft" and "field": "message-connection-id"
Reading from DeviceTree allows platforms to specify their preferred
communication channel, making it more flexible. If the property is
not found in the DeviceTree, use the default connection ID
(VMBUS_MESSAGE_CONNECTION_ID or VMBUS_MESSAGE_CONNECTION_ID_4
based on protocol version).
Signed-off-by: Hardik Garg <hargar@linux.microsoft.com>
---
v2:
- Rebased on hyperv-next branch as requested by maintainers
- Added details about the property name format in the commit message
v1:
https://lore.kernel.org/all/6acee4bf-cb04-43b9-9476-e8d811d26dfd@linux.microsoft.com/
drivers/hv/connection.c | 6 ++++--
drivers/hv/vmbus_drv.c | 13 +++++++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index be490c598785..c444c943c1d4 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -99,12 +99,14 @@ int vmbus_negotiate_version(struct
vmbus_channel_msginfo *msginfo, u32 version)
if (version >= VERSION_WIN10_V5) {
msg->msg_sint = VMBUS_MESSAGE_SINT;
msg->msg_vtl = ms_hyperv.vtl;
- vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID_4;
} else {
msg->interrupt_page = virt_to_phys(vmbus_connection.int_page);
- vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID;
}
+ /* Set default connection ID if not provided via DeviceTree */
+ if (!vmbus_connection.msg_conn_id)
+ vmbus_connection.msg_conn_id = (version >= VERSION_WIN10_V5) ?
+ VMBUS_MESSAGE_CONNECTION_ID_4 : VMBUS_MESSAGE_CONNECTION_ID;
/*
* shared_gpa_boundary is zero in non-SNP VMs, so it's safe to always
* bitwise OR it
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index c236081d0a87..6a886611c448 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2541,10 +2541,23 @@ static int vmbus_device_add(struct
platform_device *pdev)
struct of_range range;
struct of_range_parser parser;
struct device_node *np = pdev->dev.of_node;
+ unsigned int conn_id;
int ret;
vmbus_root_device = &pdev->dev;
+ /*
+ * Read connection ID from DeviceTree. The property name follows the
+ * format <vendor>,<field> where:
+ * - vendor: "microsoft"
+ * - field: "message-connection-id"
+ */
+ ret = of_property_read_u32(np, "microsoft,message-connection-id",
&conn_id);
+ if (!ret) {
+ pr_info("VMBus message connection ID: %u\n", conn_id);
+ vmbus_connection.msg_conn_id = conn_id;
+ }
+
ret = of_range_parser_init(&parser, np);
if (ret)
return ret;
--
2.40.4
^ permalink raw reply related
* RE: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Michael Kelley @ 2025-06-03 17:50 UTC (permalink / raw)
To: Thomas Zimmermann, David Hildenbrand, simona@ffwll.ch,
deller@gmx.de, haiyangz@microsoft.com, kys@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com,
akpm@linux-foundation.org
Cc: weh@microsoft.com, hch@lst.de, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, linux-mm@kvack.org
In-Reply-To: <c0b91a50-d3e7-44f9-b9c5-9c3b29639428@suse.de>
From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Monday, June 2, 2025 11:25 PM
>
> Hi
>
> Am 03.06.25 um 03:49 schrieb Michael Kelley:
> [...]
> >> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
> >> horrible hack.
> >>
> >> In another thread, you mention that you use PFN_SPECIAL to bypass the
> >> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
> > The VMA has VM_PFNMAP set, not VM_MIXEDMAP. It seemed like
> > VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
> > a wrong impression. vm_mixed_ok() does a thorough job of validating the
> > use of __vm_insert_mixed(), and since what I did was allowed, I thought
> > perhaps it was OK. Your feedback has set me straight, and that's what I
> > needed. :-)
> >
> > But the whole approach is moot with Alistair Popple's patch set that
> > eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
> > special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
> > it. If there's not one, I'll take a crack at adding it in the next version of my
> > patch set.
>
> What is the motivation behind this work? The driver or fbdev as a whole
> does not have much of a future anyway.
>
> I'd like to suggest removing hyperv_fb entirely in favor of hypervdrm?
>
Yes, I think that's the longer term direction. A couple months ago I had an
email conversation with Saurabh Sengar from the Microsoft Linux team where
he raised this idea. I think the Microsoft folks will need to drive the deprecation
process, as they need to coordinate with the distro vendors who publish
images for running on local Hyper-V and in the Azure cloud. And my
understanding is that the Linux kernel process would want the driver to
be available but marked "deprecated" for a year or so before it actually
goes away.
I do have some concerns about the maturity of the hyperv_drm driver
"around the edges". For example, somebody just recently submitted a
patch to flush output on panic. I have less familiarity hyperv_drm vs.
hyperv_fb, so some of my concern is probably due to that. We might
need to do review of hyperv_drm and see if there's anything else to
deal with before hyperv_fb goes away.
This all got started when I was looking at a problem with hyperv_fb,
and I found several other related problems, some of which also existed
in hyperv_drm. You've seen several small'ish fixes from me and Saurabh
as a result, and this issue with mmap()'ing /dev/fb0 is the last one of that
set. This fix is definitely a bit bigger, but it's the right fix. On the flip side,
if we really get on a path to deprecate hyperv_fb, there are hack fixes for
the mmap problem that are smaller and contained to hyperv_fb. I would
be OK with a hack fix in that case.
Michael
^ permalink raw reply
* RE: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Michael Kelley @ 2025-06-03 17:24 UTC (permalink / raw)
To: David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, akpm@linux-foundation.org
Cc: weh@microsoft.com, tzimmermann@suse.de, hch@lst.de,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-mm@kvack.org
In-Reply-To: <e069436f-764d-464d-98ac-36a086297632@redhat.com>
From: David Hildenbrand <david@redhat.com> Sent: Tuesday, June 3, 2025 12:55 AM
>
> On 03.06.25 03:49, Michael Kelley wrote:
> > From: David Hildenbrand <david@redhat.com> Sent: Monday, June 2, 2025 2:48 AM
> >>
> >> On 23.05.25 18:15, mhkelley58@gmail.com wrote:
> >>> From: Michael Kelley <mhklinux@outlook.com>
> >>>
> >>> Current defio code works only for framebuffer memory that is allocated
> >>> with vmalloc(). The code assumes that the underlying page refcount can
> >>> be used by the mm subsystem to manage each framebuffer page's lifecycle,
> >>> including freeing the page if the refcount goes to 0. This approach is
> >>> consistent with vmalloc'ed memory, but not with contiguous kernel memory
> >>> allocated via alloc_pages() or similar. The latter such memory pages
> >>> usually have a refcount of 0 when allocated, and would be incorrectly
> >>> freed page-by-page if used with defio. That free'ing corrupts the memory
> >>> free lists and Linux eventually panics. Simply bumping the refcount after
> >>> allocation doesn’t work because when the framebuffer memory is freed,
> >>> __free_pages() complains about non-zero refcounts.
> >>>
> >>> Commit 37b4837959cb ("video: deferred io with physically contiguous
> >>> memory") from the year 2008 purported to add support for contiguous
> >>> kernel memory framebuffers. The motivating device, sh_mobile_lcdcfb, uses
> >>> dma_alloc_coherent() to allocate framebuffer memory, which is likely to
> >>> use alloc_pages(). It's unclear to me how this commit actually worked at
> >>> the time, unless dma_alloc_coherent() was pulling from a CMA pool instead
> >>> of alloc_pages(). Or perhaps alloc_pages() worked differently or on the
> >>> arm32 architecture on which sh_mobile_lcdcfb is used.
> >>>
> >>> In any case, for x86 and arm64 today, commit 37b4837959cb9 is not
> >>> sufficient to support contiguous kernel memory framebuffers. The problem
> >>> can be seen with the hyperv_fb driver, which may allocate the framebuffer
> >>> memory using vmalloc() or alloc_pages(), depending on the configuration
> >>> of the Hyper-V guest VM (Gen 1 vs. Gen 2) and the size of the framebuffer.
> >>>
> >>> Fix this limitation by adding defio support for contiguous kernel memory
> >>> framebuffers. A driver with a framebuffer allocated from contiguous
> >>> kernel memory must set the FBINFO_KMEMFB flag to indicate such.
> >>>
> >>> Tested with the hyperv_fb driver in both configurations -- with a vmalloc()
> >>> framebuffer and with an alloc_pages() framebuffer on x86. Also verified a
> >>> vmalloc() framebuffer on arm64. Hardware is not available to me to verify
> >>> that the older arm32 devices still work correctly, but the path for
> >>> vmalloc() framebuffers is essentially unchanged.
> >>>
> >>> Even with these changes, defio does not support framebuffers in MMIO
> >>> space, as defio code depends on framebuffer memory pages having
> >>> corresponding 'struct page's.
> >>>
> >>> Fixes: 3a6fb6c4255c ("video: hyperv: hyperv_fb: Use physical memory for fb on HyperV
> Gen 1 VMs.")
> >>> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> >>> ---
> >>> Changes in v3:
> >>> * Moved definition of FBINFO_KMEMFB flag to a separate patch
> >>> preceeding this one in the patch set [Helge Deller]
> >>> Changes in v2:
> >>> * Tweaked code comments regarding framebuffers allocated with
> >>> dma_alloc_coherent() [Christoph Hellwig]
> >>>
> >>> drivers/video/fbdev/core/fb_defio.c | 128 +++++++++++++++++++++++-----
> >>> 1 file changed, 108 insertions(+), 20 deletions(-)
> >>>
> >>> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> >>> index 4fc93f253e06..f8ae91a1c4df 100644
> >>> --- a/drivers/video/fbdev/core/fb_defio.c
> >>> +++ b/drivers/video/fbdev/core/fb_defio.c
> >>> @@ -8,11 +8,40 @@
> >>> * for more details.
> >>> */
> >>>
> >>> +/*
> >>> + * Deferred I/O ("defio") allows framebuffers that are mmap()'ed to user space
> >>> + * to batch user space writes into periodic updates to the underlying
> >>> + * framebuffer hardware or other implementation (such as with a virtualized
> >>> + * framebuffer in a VM). At each batch interval, a callback is invoked in the
> >>> + * framebuffer's kernel driver, and the callback is supplied with a list of
> >>> + * pages that have been modified in the preceding interval. The callback can
> >>> + * use this information to update the framebuffer hardware as necessary. The
> >>> + * batching can improve performance and reduce the overhead of updating the
> >>> + * hardware.
> >>> + *
> >>> + * Defio is supported on framebuffers allocated using vmalloc() and allocated
> >>> + * as contiguous kernel memory using alloc_pages() or kmalloc(). These
> >>> + * memory allocations all have corresponding "struct page"s. Framebuffers
> >>> + * allocated using dma_alloc_coherent() should not be used with defio.
> >>> + * Such allocations should be treated as a black box owned by the DMA
> >>> + * layer, and should not be deconstructed into individual pages as defio
> >>> + * does. Framebuffers in MMIO space are *not* supported because MMIO space
> >>> + * does not have corrresponding "struct page"s.
> >>> + *
> >>> + * For framebuffers allocated using vmalloc(), struct fb_info must have
> >>> + * "screen_buffer" set to the vmalloc address of the framebuffer. For
> >>> + * framebuffers allocated from contiguous kernel memory, FBINFO_KMEMFB must
> >>> + * be set, and "fix.smem_start" must be set to the physical address of the
> >>> + * frame buffer. In both cases, "fix.smem_len" must be set to the framebuffer
> >>> + * size in bytes.
> >>> + */
> >>> +
> >>> #include <linux/module.h>
> >>> #include <linux/kernel.h>
> >>> #include <linux/errno.h>
> >>> #include <linux/string.h>
> >>> #include <linux/mm.h>
> >>> +#include <linux/pfn_t.h>
> >>> #include <linux/vmalloc.h>
> >>> #include <linux/delay.h>
> >>> #include <linux/interrupt.h>
> >>> @@ -37,7 +66,7 @@ static struct page *fb_deferred_io_get_page(struct fb_info *info, unsigned long
> >>> else if (info->fix.smem_start)
> >>> page = pfn_to_page((info->fix.smem_start + offs) >> PAGE_SHIFT);
> >>>
> >>> - if (page)
> >>> + if (page && !(info->flags & FBINFO_KMEMFB))
> >>> get_page(page);
> >>>
> >>> return page;
> >>> @@ -137,6 +166,15 @@ static vm_fault_t fb_deferred_io_fault(struct vm_fault *vmf)
> >>>
> >>> BUG_ON(!info->fbdefio->mapping);
> >>>
> >>> + if (info->flags & FBINFO_KMEMFB)
> >>> + /*
> >>> + * In this path, the VMA is marked VM_PFNMAP, so mm assumes
> >>> + * there is no struct page associated with the page. The
> >>> + * PFN must be directly inserted and the created PTE will be
> >>> + * marked "special".
> >>> + */
> >>> + return vmf_insert_pfn(vmf->vma, vmf->address, page_to_pfn(page));
> >>> +
> >>> vmf->page = page;
> >>> return 0;
> >>> }
> >>> @@ -163,13 +201,14 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
> >>>
> >>> /*
> >>> * Adds a page to the dirty list. Call this from struct
> >>> - * vm_operations_struct.page_mkwrite.
> >>> + * vm_operations_struct.page_mkwrite or .pfn_mkwrite.
> >>> */
> >>> -static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long offset,
> >>> +static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, struct vm_fault *vmf,
> >>> struct page *page)
> >>> {
> >>> struct fb_deferred_io *fbdefio = info->fbdefio;
> >>> struct fb_deferred_io_pageref *pageref;
> >>> + unsigned long offset = vmf->pgoff << PAGE_SHIFT;
> >>> vm_fault_t ret;
> >>>
> >>> /* protect against the workqueue changing the page list */
> >>> @@ -182,20 +221,34 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
> >>> }
> >>>
> >>> /*
> >>> - * We want the page to remain locked from ->page_mkwrite until
> >>> - * the PTE is marked dirty to avoid mapping_wrprotect_range()
> >>> - * being called before the PTE is updated, which would leave
> >>> - * the page ignored by defio.
> >>> - * Do this by locking the page here and informing the caller
> >>> - * about it with VM_FAULT_LOCKED.
> >>> + * The PTE must be marked writable before the defio deferred work runs
> >>> + * again and potentially marks the PTE write-protected. If the order
> >>> + * should be switched, the PTE would become writable without defio
> >>> + * tracking the page, leaving the page forever ignored by defio.
> >>> + *
> >>> + * For vmalloc() framebuffers, the associated struct page is locked
> >>> + * before releasing the defio lock. mm will later mark the PTE writaable
> >>> + * and release the struct page lock. The struct page lock prevents
> >>> + * the page from being prematurely being marked write-protected.
> >>> + *
> >>> + * For FBINFO_KMEMFB framebuffers, mm assumes there is no struct page,
> >>> + * so the PTE must be marked writable while the defio lock is held.
> >>> */
> >>> - lock_page(pageref->page);
> >>> + if (info->flags & FBINFO_KMEMFB) {
> >>> + unsigned long pfn = page_to_pfn(pageref->page);
> >>> +
> >>> + ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address,
> >>> + __pfn_to_pfn_t(pfn, PFN_SPECIAL));
> >>
> >> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
> >> horrible hack.
> >>
> >> In another thread, you mention that you use PFN_SPECIAL to bypass the
> >> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
> >
> > The VMA has VM_PFNMAP set, not VM_MIXEDMAP. It seemed like
> > VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
> > a wrong impression.
>
> VM_PFNMAP: nothing is refcounted except anon pages
>
> VM_MIXEDMAP: anything with a "struct page" (pfn_valid()) is refcounted
>
> pte_special() is a way for GUP-fast to distinguish these refcounted (can
> GUP) from non-refcounted (camnnot GUP) pages mapped by PTEs without any
> locks or the VMA being available.
>
> Setting pte_special() in VM_MIXEDMAP on ptes that have a "struct page"
> (pfn_valid()) is likely very bogus.
OK, good to know.
>
> > vm_mixed_ok() does a thorough job of validating the
> > use of __vm_insert_mixed(), and since what I did was allowed, I thought
> > perhaps it was OK. Your feedback has set me straight, and that's what I
> > needed. :-)
>
> What exactly are you trying to achieve? :)
>
> If it's mapping a page with a "struct page" and *not* refcounting it,
> then vmf_insert_pfn() is the current way to achieve that in a VM_PFNMAP
> mapping. It will set pte_special() automatically for you.
>
Yes, that's what I'm using to initially create the special PTE in the
.fault callback.
> >
> > But the whole approach is moot with Alistair Popple's patch set that
> > eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
> > special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
> > it. If there's not one, I'll take a crack at adding it in the next version of my
> > patch set.
>
> I assume you'd want vmf_insert_pfn_mkwrite(), correct? Probably
> vmf_insert_pfn_prot() can be used by adding PAGE_WRITE to pgprot. (maybe
> :) )
Ok, I'll look at that more closely. The sequence is that the special
PTE gets created with vmf_insert_pfn(). Then when the page is first
written to, the .pfn_mkwrite callback is invoked by mm. The question
is the best way for that callback to mark the existing PTE as writable.
Thanks,
Michael
^ permalink raw reply
* Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions
From: Josh Poimboeuf @ 2025-06-03 16:29 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Sean Christopherson, H. Peter Anvin, x86, kys, haiyangz, wei.liu,
decui, tglx, mingo, bp, dave.hansen, pbonzini, ardb, kees,
Arnd Bergmann, gregkh, linux-hyperv, linux-kernel, kvm, linux-efi,
samitolvanen, ojeda, xin
In-Reply-To: <fp5amaygv37wxr6bglagljr325rsagllbabb62ow44kl3mznb6@gzk6nuukjgwv>
On Mon, Jun 02, 2025 at 10:43:42PM -0700, Josh Poimboeuf wrote:
> On Thu, May 29, 2025 at 11:30:17AM +0200, Peter Zijlstra wrote:
> > > > So the sequence of fail is:
> > > >
> > > > push %rbp
> > > > mov %rsp, %rbp # cfa.base = BP
> > > >
> > > > SAVE
> >
> > sub $0x40,%rsp
> > and $0xffffffffffffffc0,%rsp
> >
> > This hits the 'older GCC, drap with frame pointer' case in OP_SRC_AND.
> > Which means we then hard rely on the frame pointer to get things right.
> >
> > However, per all the PUSH/POP_REGS nonsense, BP can get clobbered.
> > Specifically the code between the CALL and POP %rbp below are up in the
> > air. I don't think it can currently unwind properly there.
>
> RBP is callee saved, so there's no need to pop it or any of the other
> callee-saved regs. If they were to change, that would break C ABI
> pretty badly. Maybe add a skip_callee=1 arg to POP_REGS?
This compiles for me:
diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index d83236b96f22..414f8bcf07ec 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -99,7 +99,7 @@ For 32-bit we have the following conventions - kernel is built with
.endif
.endm
-.macro CLEAR_REGS clear_bp=1
+.macro CLEAR_REGS clear_callee=1
/*
* Sanitize registers of values that a speculation attack might
* otherwise want to exploit. The lower registers are likely clobbered
@@ -113,29 +113,31 @@ For 32-bit we have the following conventions - kernel is built with
xorl %r9d, %r9d /* nospec r9 */
xorl %r10d, %r10d /* nospec r10 */
xorl %r11d, %r11d /* nospec r11 */
+ .if \clear_callee
xorl %ebx, %ebx /* nospec rbx */
- .if \clear_bp
xorl %ebp, %ebp /* nospec rbp */
- .endif
xorl %r12d, %r12d /* nospec r12 */
xorl %r13d, %r13d /* nospec r13 */
xorl %r14d, %r14d /* nospec r14 */
xorl %r15d, %r15d /* nospec r15 */
+ .endif
.endm
-.macro PUSH_AND_CLEAR_REGS rdx=%rdx rcx=%rcx rax=%rax save_ret=0 clear_bp=1 unwind_hint=1
+.macro PUSH_AND_CLEAR_REGS rdx=%rdx rcx=%rcx rax=%rax save_ret=0 clear_callee=1 unwind_hint=1
PUSH_REGS rdx=\rdx, rcx=\rcx, rax=\rax, save_ret=\save_ret unwind_hint=\unwind_hint
- CLEAR_REGS clear_bp=\clear_bp
+ CLEAR_REGS clear_callee=\clear_callee
.endm
-.macro POP_REGS pop_rdi=1
+.macro POP_REGS pop_rdi=1 pop_callee=1
+.if \pop_callee
popq %r15
popq %r14
popq %r13
popq %r12
popq %rbp
popq %rbx
+.endif
popq %r11
popq %r10
popq %r9
diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
index 29c5c32c16c3..277f980c46fd 100644
--- a/arch/x86/entry/entry_64_fred.S
+++ b/arch/x86/entry/entry_64_fred.S
@@ -112,11 +112,12 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
push %rax /* Return RIP */
push $0 /* Error code, 0 for IRQ/NMI */
- PUSH_AND_CLEAR_REGS clear_bp=0 unwind_hint=0
+ PUSH_AND_CLEAR_REGS clear_callee=0 unwind_hint=0
movq %rsp, %rdi /* %rdi -> pt_regs */
call __fred_entry_from_kvm /* Call the C entry point */
- POP_REGS
- ERETS
+ POP_REGS pop_callee=0
+
+ ALTERNATIVE "mov %rbp, %rsp", __stringify(ERETS), X86_FEATURE_FRED
1:
/*
* Objtool doesn't understand what ERETS does, this hint tells it that
^ permalink raw reply related
* Re: [GIT PULL] Hyper-V patches for 6.16
From: pr-tracker-bot @ 2025-06-03 16:09 UTC (permalink / raw)
To: Wei Liu
Cc: Linus Torvalds, Wei Liu, Linux on Hyper-V List, Linux Kernel List,
kys, haiyangz, decui
In-Reply-To: <aD6Rq7Hm499y5ybR@liuwe-devbox-ubuntu-v2.tail21d00.ts.net>
The pull request you sent on Tue, 3 Jun 2025 06:09:47 +0000:
> ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-next-signed-20250602
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/c00b28502470b7677c2f4ee2e9c899205fa78e27
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: David Hildenbrand @ 2025-06-03 7:55 UTC (permalink / raw)
To: Michael Kelley, simona@ffwll.ch, deller@gmx.de,
haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, akpm@linux-foundation.org
Cc: weh@microsoft.com, tzimmermann@suse.de, hch@lst.de,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-mm@kvack.org
In-Reply-To: <SN6PR02MB41573C075152ECD8428CAF5ED46DA@SN6PR02MB4157.namprd02.prod.outlook.com>
On 03.06.25 03:49, Michael Kelley wrote:
> From: David Hildenbrand <david@redhat.com> Sent: Monday, June 2, 2025 2:48 AM
>>
>> On 23.05.25 18:15, mhkelley58@gmail.com wrote:
>>> From: Michael Kelley <mhklinux@outlook.com>
>>>
>>> Current defio code works only for framebuffer memory that is allocated
>>> with vmalloc(). The code assumes that the underlying page refcount can
>>> be used by the mm subsystem to manage each framebuffer page's lifecycle,
>>> including freeing the page if the refcount goes to 0. This approach is
>>> consistent with vmalloc'ed memory, but not with contiguous kernel memory
>>> allocated via alloc_pages() or similar. The latter such memory pages
>>> usually have a refcount of 0 when allocated, and would be incorrectly
>>> freed page-by-page if used with defio. That free'ing corrupts the memory
>>> free lists and Linux eventually panics. Simply bumping the refcount after
>>> allocation doesn’t work because when the framebuffer memory is freed,
>>> __free_pages() complains about non-zero refcounts.
>>>
>>> Commit 37b4837959cb ("video: deferred io with physically contiguous
>>> memory") from the year 2008 purported to add support for contiguous
>>> kernel memory framebuffers. The motivating device, sh_mobile_lcdcfb, uses
>>> dma_alloc_coherent() to allocate framebuffer memory, which is likely to
>>> use alloc_pages(). It's unclear to me how this commit actually worked at
>>> the time, unless dma_alloc_coherent() was pulling from a CMA pool instead
>>> of alloc_pages(). Or perhaps alloc_pages() worked differently or on the
>>> arm32 architecture on which sh_mobile_lcdcfb is used.
>>>
>>> In any case, for x86 and arm64 today, commit 37b4837959cb9 is not
>>> sufficient to support contiguous kernel memory framebuffers. The problem
>>> can be seen with the hyperv_fb driver, which may allocate the framebuffer
>>> memory using vmalloc() or alloc_pages(), depending on the configuration
>>> of the Hyper-V guest VM (Gen 1 vs. Gen 2) and the size of the framebuffer.
>>>
>>> Fix this limitation by adding defio support for contiguous kernel memory
>>> framebuffers. A driver with a framebuffer allocated from contiguous
>>> kernel memory must set the FBINFO_KMEMFB flag to indicate such.
>>>
>>> Tested with the hyperv_fb driver in both configurations -- with a vmalloc()
>>> framebuffer and with an alloc_pages() framebuffer on x86. Also verified a
>>> vmalloc() framebuffer on arm64. Hardware is not available to me to verify
>>> that the older arm32 devices still work correctly, but the path for
>>> vmalloc() framebuffers is essentially unchanged.
>>>
>>> Even with these changes, defio does not support framebuffers in MMIO
>>> space, as defio code depends on framebuffer memory pages having
>>> corresponding 'struct page's.
>>>
>>> Fixes: 3a6fb6c4255c ("video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.")
>>> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
>>> ---
>>> Changes in v3:
>>> * Moved definition of FBINFO_KMEMFB flag to a separate patch
>>> preceeding this one in the patch set [Helge Deller]
>>> Changes in v2:
>>> * Tweaked code comments regarding framebuffers allocated with
>>> dma_alloc_coherent() [Christoph Hellwig]
>>>
>>> drivers/video/fbdev/core/fb_defio.c | 128 +++++++++++++++++++++++-----
>>> 1 file changed, 108 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
>>> index 4fc93f253e06..f8ae91a1c4df 100644
>>> --- a/drivers/video/fbdev/core/fb_defio.c
>>> +++ b/drivers/video/fbdev/core/fb_defio.c
>>> @@ -8,11 +8,40 @@
>>> * for more details.
>>> */
>>>
>>> +/*
>>> + * Deferred I/O ("defio") allows framebuffers that are mmap()'ed to user space
>>> + * to batch user space writes into periodic updates to the underlying
>>> + * framebuffer hardware or other implementation (such as with a virtualized
>>> + * framebuffer in a VM). At each batch interval, a callback is invoked in the
>>> + * framebuffer's kernel driver, and the callback is supplied with a list of
>>> + * pages that have been modified in the preceding interval. The callback can
>>> + * use this information to update the framebuffer hardware as necessary. The
>>> + * batching can improve performance and reduce the overhead of updating the
>>> + * hardware.
>>> + *
>>> + * Defio is supported on framebuffers allocated using vmalloc() and allocated
>>> + * as contiguous kernel memory using alloc_pages() or kmalloc(). These
>>> + * memory allocations all have corresponding "struct page"s. Framebuffers
>>> + * allocated using dma_alloc_coherent() should not be used with defio.
>>> + * Such allocations should be treated as a black box owned by the DMA
>>> + * layer, and should not be deconstructed into individual pages as defio
>>> + * does. Framebuffers in MMIO space are *not* supported because MMIO space
>>> + * does not have corrresponding "struct page"s.
>>> + *
>>> + * For framebuffers allocated using vmalloc(), struct fb_info must have
>>> + * "screen_buffer" set to the vmalloc address of the framebuffer. For
>>> + * framebuffers allocated from contiguous kernel memory, FBINFO_KMEMFB must
>>> + * be set, and "fix.smem_start" must be set to the physical address of the
>>> + * frame buffer. In both cases, "fix.smem_len" must be set to the framebuffer
>>> + * size in bytes.
>>> + */
>>> +
>>> #include <linux/module.h>
>>> #include <linux/kernel.h>
>>> #include <linux/errno.h>
>>> #include <linux/string.h>
>>> #include <linux/mm.h>
>>> +#include <linux/pfn_t.h>
>>> #include <linux/vmalloc.h>
>>> #include <linux/delay.h>
>>> #include <linux/interrupt.h>
>>> @@ -37,7 +66,7 @@ static struct page *fb_deferred_io_get_page(struct fb_info *info, unsigned long
>>> else if (info->fix.smem_start)
>>> page = pfn_to_page((info->fix.smem_start + offs) >> PAGE_SHIFT);
>>>
>>> - if (page)
>>> + if (page && !(info->flags & FBINFO_KMEMFB))
>>> get_page(page);
>>>
>>> return page;
>>> @@ -137,6 +166,15 @@ static vm_fault_t fb_deferred_io_fault(struct vm_fault *vmf)
>>>
>>> BUG_ON(!info->fbdefio->mapping);
>>>
>>> + if (info->flags & FBINFO_KMEMFB)
>>> + /*
>>> + * In this path, the VMA is marked VM_PFNMAP, so mm assumes
>>> + * there is no struct page associated with the page. The
>>> + * PFN must be directly inserted and the created PTE will be
>>> + * marked "special".
>>> + */
>>> + return vmf_insert_pfn(vmf->vma, vmf->address, page_to_pfn(page));
>>> +
>>> vmf->page = page;
>>> return 0;
>>> }
>>> @@ -163,13 +201,14 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
>>>
>>> /*
>>> * Adds a page to the dirty list. Call this from struct
>>> - * vm_operations_struct.page_mkwrite.
>>> + * vm_operations_struct.page_mkwrite or .pfn_mkwrite.
>>> */
>>> -static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long offset,
>>> +static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, struct vm_fault *vmf,
>>> struct page *page)
>>> {
>>> struct fb_deferred_io *fbdefio = info->fbdefio;
>>> struct fb_deferred_io_pageref *pageref;
>>> + unsigned long offset = vmf->pgoff << PAGE_SHIFT;
>>> vm_fault_t ret;
>>>
>>> /* protect against the workqueue changing the page list */
>>> @@ -182,20 +221,34 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
>>> }
>>>
>>> /*
>>> - * We want the page to remain locked from ->page_mkwrite until
>>> - * the PTE is marked dirty to avoid mapping_wrprotect_range()
>>> - * being called before the PTE is updated, which would leave
>>> - * the page ignored by defio.
>>> - * Do this by locking the page here and informing the caller
>>> - * about it with VM_FAULT_LOCKED.
>>> + * The PTE must be marked writable before the defio deferred work runs
>>> + * again and potentially marks the PTE write-protected. If the order
>>> + * should be switched, the PTE would become writable without defio
>>> + * tracking the page, leaving the page forever ignored by defio.
>>> + *
>>> + * For vmalloc() framebuffers, the associated struct page is locked
>>> + * before releasing the defio lock. mm will later mark the PTE writaable
>>> + * and release the struct page lock. The struct page lock prevents
>>> + * the page from being prematurely being marked write-protected.
>>> + *
>>> + * For FBINFO_KMEMFB framebuffers, mm assumes there is no struct page,
>>> + * so the PTE must be marked writable while the defio lock is held.
>>> */
>>> - lock_page(pageref->page);
>>> + if (info->flags & FBINFO_KMEMFB) {
>>> + unsigned long pfn = page_to_pfn(pageref->page);
>>> +
>>> + ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address,
>>> + __pfn_to_pfn_t(pfn, PFN_SPECIAL));
>>
>> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
>> horrible hack.
>>
>> In another thread, you mention that you use PFN_SPECIAL to bypass the
>> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
>
> The VMA has VM_PFNMAP set, not VM_MIXEDMAP. It seemed like
> VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
> a wrong impression.
VM_PFNMAP: nothing is refcounted except anon pages
VM_MIXEDMAP: anything with a "struct page" (pfn_valid()) is refcounted
pte_special() is a way for GUP-fast to distinguish these refcounted (can
GUP) from non-refcounted (camnnot GUP) pages mapped by PTEs without any
locks or the VMA being available.
Setting pte_special() in VM_MIXEDMAP on ptes that have a "struct page"
(pfn_valid()) is likely very bogus.
> vm_mixed_ok() does a thorough job of validating the
> use of __vm_insert_mixed(), and since what I did was allowed, I thought
> perhaps it was OK. Your feedback has set me straight, and that's what I
> needed. :-)
What exactly are you trying to achieve? :)
If it's mapping a page with a "struct page" and *not* refcounting it,
then vmf_insert_pfn() is the current way to achieve that in a VM_PFNMAP
mapping. It will set pte_special() automatically for you.
>
> But the whole approach is moot with Alistair Popple's patch set that
> eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
> special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
> it. If there's not one, I'll take a crack at adding it in the next version of my
> patch set.
I assume you'd want vmf_insert_pfn_mkwrite(), correct? Probably
vmf_insert_pfn_prot() can be used by adding PAGE_WRITE to pgprot. (maybe
:) )
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Thomas Zimmermann @ 2025-06-03 6:25 UTC (permalink / raw)
To: Michael Kelley, David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, akpm@linux-foundation.org
Cc: weh@microsoft.com, hch@lst.de, dri-devel@lists.freedesktop.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, linux-mm@kvack.org
In-Reply-To: <SN6PR02MB41573C075152ECD8428CAF5ED46DA@SN6PR02MB4157.namprd02.prod.outlook.com>
Hi
Am 03.06.25 um 03:49 schrieb Michael Kelley:
[...]
>> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
>> horrible hack.
>>
>> In another thread, you mention that you use PFN_SPECIAL to bypass the
>> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
> The VMA has VM_PFNMAP set, not VM_MIXEDMAP. It seemed like
> VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
> a wrong impression. vm_mixed_ok() does a thorough job of validating the
> use of __vm_insert_mixed(), and since what I did was allowed, I thought
> perhaps it was OK. Your feedback has set me straight, and that's what I
> needed. :-)
>
> But the whole approach is moot with Alistair Popple's patch set that
> eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
> special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
> it. If there's not one, I'll take a crack at adding it in the next version of my
> patch set.
What is the motivation behind this work? The driver or fbdev as a whole
does not have much of a future anyway.
I'd like to suggest removing hyperv_fb entirely in favor of hypervdrm?
Best regards
Thomas
>
> Michael
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* [GIT PULL] Hyper-V patches for 6.16
From: Wei Liu @ 2025-06-03 6:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: Wei Liu, Linux on Hyper-V List, Linux Kernel List, kys, haiyangz,
decui
Hi Linus,
The following changes since commit 82f2b0b97b36ee3fcddf0f0780a9a0825d52fec3:
Linux 6.15-rc6 (2025-05-11 14:54:11 -0700)
are available in the Git repository at:
ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-next-signed-20250602
for you to fetch changes up to 96959283a58d91ae20d025546f00e16f0a555208:
Drivers: hv: Always select CONFIG_SYSFB for Hyper-V guests (2025-05-23 16:39:37 +0000)
You will see a conflict when you merge this tag. A resolution can be
found here:
https://lore.kernel.org/linux-next/20250515181414.354d8ef7@canb.auug.org.au/
----------------------------------------------------------------
hyperv-next for v6.16
- Support for Virtual Trust Level (VTL) on arm64 (Roman Kisel)
- Fixes for Hyper-V UIO driver (Long Li)
- Fixes for Hyper-V PCI driver (Michael Kelley)
- Select CONFIG_SYSFB for Hyper-V guests (Michael Kelley)
- Documentation updates for Hyper-V VMBus (Michael Kelley)
- Enhance logging for hv_kvp_daemon (Shradha Gupta)
----------------------------------------------------------------
Long Li (5):
Drivers: hv: Allocate interrupt and monitor pages aligned to system page boundary
uio_hv_generic: Use correct size for interrupt and monitor pages
uio_hv_generic: Align ring size to system page
Drivers: hv: Use kzalloc for panic page allocation
Drivers: hv: Remove hv_alloc/free_* helpers
Michael Kelley (4):
PCI: hv: Remove unnecessary flex array in struct pci_packet
Documentation: hyperv: Update VMBus doc with new features and info
Drivers: hv: vmbus: Add comments about races with "channels" sysfs dir
Drivers: hv: Always select CONFIG_SYSFB for Hyper-V guests
Roman Kisel (13):
arm64: kvm, smccc: Introduce and use API for getting hypervisor UUID
arm64: hyperv: Use SMCCC to detect hypervisor presence
Drivers: hv: Enable VTL mode for arm64
Drivers: hv: Provide arch-neutral implementation of get_vtl()
arm64: hyperv: Initialize the Virtual Trust Level field
arm64, x86: hyperv: Report the VTL the system boots in
dt-bindings: microsoft,vmbus: Add interrupt and DMA coherence properties
Drivers: hv: vmbus: Get the IRQ number from DeviceTree
Drivers: hv: vmbus: Introduce hv_get_vmbus_root_device()
ACPI: irq: Introduce acpi_get_gsi_dispatcher()
PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
x86/hyperv: Fix APIC ID and VP index confusion in hv_snp_boot_ap()
arch/x86: Provide the CPU number in the wakeup AP callback
Shradha Gupta (1):
tools: hv: Enable debug logs for hv_kvp_daemon
.../devicetree/bindings/bus/microsoft,vmbus.yaml | 16 +++-
Documentation/virt/hyperv/vmbus.rst | 28 +++++-
arch/arm64/hyperv/mshyperv.c | 53 ++++++++++--
arch/arm64/kvm/hypercalls.c | 10 ++-
arch/x86/coco/sev/core.c | 13 +--
arch/x86/hyperv/hv_init.c | 67 ++++++++-------
arch/x86/hyperv/hv_vtl.c | 61 +++----------
arch/x86/hyperv/ivm.c | 11 ++-
arch/x86/include/asm/apic.h | 8 +-
arch/x86/include/asm/mshyperv.h | 7 +-
arch/x86/kernel/acpi/madt_wakeup.c | 2 +-
arch/x86/kernel/apic/apic_noop.c | 8 +-
arch/x86/kernel/apic/apic_numachip.c | 2 +-
arch/x86/kernel/apic/x2apic_uv_x.c | 2 +-
arch/x86/kernel/smpboot.c | 10 +--
drivers/acpi/irq.c | 16 +++-
drivers/firmware/smccc/kvm_guest.c | 10 +--
drivers/firmware/smccc/smccc.c | 17 ++++
drivers/hv/Kconfig | 7 +-
drivers/hv/connection.c | 23 +++--
drivers/hv/hv_common.c | 76 ++++++++---------
drivers/hv/vmbus_drv.c | 95 ++++++++++++++++++---
drivers/pci/controller/pci-hyperv.c | 99 +++++++++++++++++-----
drivers/uio/uio_hv_generic.c | 7 +-
include/asm-generic/mshyperv.h | 10 ++-
include/hyperv/hvgdk_mini.h | 4 +-
include/linux/acpi.h | 5 +-
include/linux/arm-smccc.h | 64 +++++++++++++-
include/linux/hyperv.h | 2 +
tools/hv/hv_kvp_daemon.c | 64 ++++++++++++--
30 files changed, 562 insertions(+), 235 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions
From: Josh Poimboeuf @ 2025-06-03 5:43 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Sean Christopherson, H. Peter Anvin, x86, kys, haiyangz, wei.liu,
decui, tglx, mingo, bp, dave.hansen, pbonzini, ardb, kees,
Arnd Bergmann, gregkh, linux-hyperv, linux-kernel, kvm, linux-efi,
samitolvanen, ojeda, xin
In-Reply-To: <20250529093017.GJ31726@noisy.programming.kicks-ass.net>
On Thu, May 29, 2025 at 11:30:17AM +0200, Peter Zijlstra wrote:
> > > So the sequence of fail is:
> > >
> > > push %rbp
> > > mov %rsp, %rbp # cfa.base = BP
> > >
> > > SAVE
>
> sub $0x40,%rsp
> and $0xffffffffffffffc0,%rsp
>
> This hits the 'older GCC, drap with frame pointer' case in OP_SRC_AND.
> Which means we then hard rely on the frame pointer to get things right.
>
> However, per all the PUSH/POP_REGS nonsense, BP can get clobbered.
> Specifically the code between the CALL and POP %rbp below are up in the
> air. I don't think it can currently unwind properly there.
RBP is callee saved, so there's no need to pop it or any of the other
callee-saved regs. If they were to change, that would break C ABI
pretty badly. Maybe add a skip_callee=1 arg to POP_REGS?
--
Josh
^ permalink raw reply
* Re: [PATCH v4 0/5] Allow dyn MSI-X vector allocation of MANA
From: Shradha Gupta @ 2025-06-03 4:17 UTC (permalink / raw)
To: Zhu Yanjun
Cc: linux-hyperv, linux-pci, linux-kernel, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy???~Dski,
Lorenzo Pieralisi, Dexuan Cui, Wei Liu, Haiyang Zhang,
K. Y. Srinivasan, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Konstantin Taranov, Simon Horman,
Leon Romanovsky, Maxim Levitsky, Erni Sri Satya Vennela,
Peter Zijlstra, netdev, linux-rdma, Paul Rosswurm, Shradha Gupta
In-Reply-To: <83244641-8fab-4f05-9d31-c5881fa1660c@linux.dev>
On Sun, Jun 01, 2025 at 04:53:31PM +0200, Zhu Yanjun wrote:
> ??? 2025/5/27 17:57, Shradha Gupta ??????:
> >In this patchset we want to enable the MANA driver to be able to
> >allocate MSI-X vectors in PCI dynamically.
> >
> >The first patch exports pci_msix_prepare_desc() in PCI to be able to
> >correctly prepare descriptors for dynamically added MSI-X vectors.
> >
> >The second patch adds the support of dynamic vector allocation in
> >pci-hyperv PCI controller by enabling the MSI_FLAG_PCI_MSIX_ALLOC_DYN
> >flag and using the pci_msix_prepare_desc() exported in first patch.
> >
> >The third patch adds a detailed description of the irq_setup(), to
> >help understand the function design better.
> >
> >The fourth patch is a preparation patch for mana changes to support
> >dynamic IRQ allocation. It contains changes in irq_setup() to allow
> >skipping first sibling CPU sets, in case certain IRQs are already
> >affinitized to them.
> >
> >The fifth patch has the changes in MANA driver to be able to allocate
> >MSI-X vectors dynamically. If the support does not exist it defaults to
> >older behavior.
> >---
> > Change in v4
> > * add a patch describing the functionality of irq_setup() through a
> > comment
> > * In irq_setup(), avoid using a label next_cpumask:
> > * modify the changes in MANA patch about restructuring the error
> > handling path in mana_gd_setup_dyn_irqs()
> > * modify the mana_gd_setup_irqs() to simplify handling around
> > start_irq_index
> > * add warning if an invalid gic is returned
> > * place the xa_destroy() cleanup in mana_gd_remove
> >---
> > Changes in v3
> > * split the 3rd patch into preparation patch around irq_setup() and
> > changes in mana driver to allow dynamic IRQ allocation
> > * Add arm64 support for dynamic MSI-X allocation in pci_hyperv
> > controller
> >---
> > Changes in v2
> > * split the first patch into two(exporting the preapre_desc
> > func and using the function and flag in pci-hyperv)
> > * replace 'pci vectors' by 'MSI-X vectors'
> > * Change the cover letter description to align with changes made
> >---
> >
> >Shradha Gupta (5):
> > PCI/MSI: Export pci_msix_prepare_desc() for dynamic MSI-X allocations
> > PCI: hv: Allow dynamic MSI-X vector allocation
> > net: mana: explain irq_setup() algorithm
> > net: mana: Allow irq_setup() to skip cpus for affinity
> > net: mana: Allocate MSI-X vectors dynamically
>
> In this patchset, base-commit seems missing.
>
> Please see this link:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.15#n868
>
> "
> When you open ``outgoing/0000-cover-letter.patch`` for editing, you will
> notice that it will have the ``base-commit:`` trailer at the very
> bottom, which provides the reviewer and the CI tools enough information
> to properly perform ``git am`` without worrying about conflicts::
> "
>
> When creating patches:
> "
> git format-patch --base=main origin/main
> "
>
> This will include a base-commit: line in each patch file:
>
> "
> base-commit: abcdef1234567890...
> "
>
> This is useful when submitting patches to mailing lists or other tooling.
>
> Please follow the submitting-patches.rst to add base-commit.
>
> Best Regards,
> Zhu Yanjun
>
Thank you, I will make the necessary changes in the next version.
Regards,
Shradha.
> >
> > .../net/ethernet/microsoft/mana/gdma_main.c | 356 ++++++++++++++----
> > drivers/pci/controller/pci-hyperv.c | 5 +-
> > drivers/pci/msi/irqdomain.c | 5 +-
> > include/linux/msi.h | 2 +
> > include/net/mana/gdma.h | 8 +-
> > 5 files changed, 293 insertions(+), 83 deletions(-)
> >
^ permalink raw reply
* Re: [PATCH v4 0/5] Allow dyn MSI-X vector allocation of MANA
From: Shradha Gupta @ 2025-06-03 4:15 UTC (permalink / raw)
To: Simon Horman
Cc: linux-hyperv, linux-pci, linux-kernel, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy???~Dski,
Lorenzo Pieralisi, Dexuan Cui, Wei Liu, Haiyang Zhang,
K. Y. Srinivasan, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Konstantin Taranov, Leon Romanovsky,
Maxim Levitsky, Erni Sri Satya Vennela, Peter Zijlstra, netdev,
linux-rdma, Paul Rosswurm, Shradha Gupta
In-Reply-To: <20250530180732.GS1484967@horms.kernel.org>
On Fri, May 30, 2025 at 07:07:32PM +0100, Simon Horman wrote:
> On Thu, May 29, 2025 at 06:28:45AM -0700, Shradha Gupta wrote:
> > On Wed, May 28, 2025 at 07:55:08PM +0100, Simon Horman wrote:
> > > On Tue, May 27, 2025 at 08:57:33AM -0700, Shradha Gupta wrote:
> > > > In this patchset we want to enable the MANA driver to be able to
> > > > allocate MSI-X vectors in PCI dynamically.
> > > >
> > > > The first patch exports pci_msix_prepare_desc() in PCI to be able to
> > > > correctly prepare descriptors for dynamically added MSI-X vectors.
> > > >
> > > > The second patch adds the support of dynamic vector allocation in
> > > > pci-hyperv PCI controller by enabling the MSI_FLAG_PCI_MSIX_ALLOC_DYN
> > > > flag and using the pci_msix_prepare_desc() exported in first patch.
> > > >
> > > > The third patch adds a detailed description of the irq_setup(), to
> > > > help understand the function design better.
> > > >
> > > > The fourth patch is a preparation patch for mana changes to support
> > > > dynamic IRQ allocation. It contains changes in irq_setup() to allow
> > > > skipping first sibling CPU sets, in case certain IRQs are already
> > > > affinitized to them.
> > > >
> > > > The fifth patch has the changes in MANA driver to be able to allocate
> > > > MSI-X vectors dynamically. If the support does not exist it defaults to
> > > > older behavior.
> > >
> > > Hi Shradha,
> > >
> > > It's unclear what the target tree for this patch-set is.
> > > But if it is net-next, which seems likely given the code under
> > > drivers/net/, then:
> > >
> > > Please include that target in the subject of each patch in the patch-set.
> > >
> > > Subject: [PATCH v5 net-next 0/5] ...
> > >
> > > And, moreover, ...
> > >
> > > ## Form letter - net-next-closed
> > >
> > > The merge window for v6.16 has begun and therefore net-next is closed
> > > for new drivers, features, code refactoring and optimizations. We are
> > > currently accepting bug fixes only.
> > >
> > > Please repost when net-next reopens after June 8th.
> > >
> > > RFC patches sent for review only are obviously welcome at any time.
> >
> > Thank you Simon.
> >
> > While posting this patchset I was a bit confused about what should be
> > the target tree. That's why in the cover letter of the V1 for this
> > series, I had requested more clarity on the same (since there are patches
> > from PCI and net-next both).
> >
> > In such cases how do we decide which tree to target?
>
> Yes, that isn't entirely clear to me either.
> Hopefully the maintainers can negotiate this.
>
Thanks Simon. also since teh target tree is not entirely clear, can I
still send out an updated version with suggested changes?
> >
> > Also, noted about the next merge window for net-next :-)
> >
> > Regards,
> > Shradha.
> >
^ permalink raw reply
* RE: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Michael Kelley @ 2025-06-03 1:49 UTC (permalink / raw)
To: David Hildenbrand, simona@ffwll.ch, deller@gmx.de,
haiyangz@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, akpm@linux-foundation.org
Cc: weh@microsoft.com, tzimmermann@suse.de, hch@lst.de,
dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-mm@kvack.org
In-Reply-To: <de0f2cb8-aed6-436f-b55e-d3f7b3fe6d81@redhat.com>
From: David Hildenbrand <david@redhat.com> Sent: Monday, June 2, 2025 2:48 AM
>
> On 23.05.25 18:15, mhkelley58@gmail.com wrote:
> > From: Michael Kelley <mhklinux@outlook.com>
> >
> > Current defio code works only for framebuffer memory that is allocated
> > with vmalloc(). The code assumes that the underlying page refcount can
> > be used by the mm subsystem to manage each framebuffer page's lifecycle,
> > including freeing the page if the refcount goes to 0. This approach is
> > consistent with vmalloc'ed memory, but not with contiguous kernel memory
> > allocated via alloc_pages() or similar. The latter such memory pages
> > usually have a refcount of 0 when allocated, and would be incorrectly
> > freed page-by-page if used with defio. That free'ing corrupts the memory
> > free lists and Linux eventually panics. Simply bumping the refcount after
> > allocation doesn’t work because when the framebuffer memory is freed,
> > __free_pages() complains about non-zero refcounts.
> >
> > Commit 37b4837959cb ("video: deferred io with physically contiguous
> > memory") from the year 2008 purported to add support for contiguous
> > kernel memory framebuffers. The motivating device, sh_mobile_lcdcfb, uses
> > dma_alloc_coherent() to allocate framebuffer memory, which is likely to
> > use alloc_pages(). It's unclear to me how this commit actually worked at
> > the time, unless dma_alloc_coherent() was pulling from a CMA pool instead
> > of alloc_pages(). Or perhaps alloc_pages() worked differently or on the
> > arm32 architecture on which sh_mobile_lcdcfb is used.
> >
> > In any case, for x86 and arm64 today, commit 37b4837959cb9 is not
> > sufficient to support contiguous kernel memory framebuffers. The problem
> > can be seen with the hyperv_fb driver, which may allocate the framebuffer
> > memory using vmalloc() or alloc_pages(), depending on the configuration
> > of the Hyper-V guest VM (Gen 1 vs. Gen 2) and the size of the framebuffer.
> >
> > Fix this limitation by adding defio support for contiguous kernel memory
> > framebuffers. A driver with a framebuffer allocated from contiguous
> > kernel memory must set the FBINFO_KMEMFB flag to indicate such.
> >
> > Tested with the hyperv_fb driver in both configurations -- with a vmalloc()
> > framebuffer and with an alloc_pages() framebuffer on x86. Also verified a
> > vmalloc() framebuffer on arm64. Hardware is not available to me to verify
> > that the older arm32 devices still work correctly, but the path for
> > vmalloc() framebuffers is essentially unchanged.
> >
> > Even with these changes, defio does not support framebuffers in MMIO
> > space, as defio code depends on framebuffer memory pages having
> > corresponding 'struct page's.
> >
> > Fixes: 3a6fb6c4255c ("video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.")
> > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> > ---
> > Changes in v3:
> > * Moved definition of FBINFO_KMEMFB flag to a separate patch
> > preceeding this one in the patch set [Helge Deller]
> > Changes in v2:
> > * Tweaked code comments regarding framebuffers allocated with
> > dma_alloc_coherent() [Christoph Hellwig]
> >
> > drivers/video/fbdev/core/fb_defio.c | 128 +++++++++++++++++++++++-----
> > 1 file changed, 108 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> > index 4fc93f253e06..f8ae91a1c4df 100644
> > --- a/drivers/video/fbdev/core/fb_defio.c
> > +++ b/drivers/video/fbdev/core/fb_defio.c
> > @@ -8,11 +8,40 @@
> > * for more details.
> > */
> >
> > +/*
> > + * Deferred I/O ("defio") allows framebuffers that are mmap()'ed to user space
> > + * to batch user space writes into periodic updates to the underlying
> > + * framebuffer hardware or other implementation (such as with a virtualized
> > + * framebuffer in a VM). At each batch interval, a callback is invoked in the
> > + * framebuffer's kernel driver, and the callback is supplied with a list of
> > + * pages that have been modified in the preceding interval. The callback can
> > + * use this information to update the framebuffer hardware as necessary. The
> > + * batching can improve performance and reduce the overhead of updating the
> > + * hardware.
> > + *
> > + * Defio is supported on framebuffers allocated using vmalloc() and allocated
> > + * as contiguous kernel memory using alloc_pages() or kmalloc(). These
> > + * memory allocations all have corresponding "struct page"s. Framebuffers
> > + * allocated using dma_alloc_coherent() should not be used with defio.
> > + * Such allocations should be treated as a black box owned by the DMA
> > + * layer, and should not be deconstructed into individual pages as defio
> > + * does. Framebuffers in MMIO space are *not* supported because MMIO space
> > + * does not have corrresponding "struct page"s.
> > + *
> > + * For framebuffers allocated using vmalloc(), struct fb_info must have
> > + * "screen_buffer" set to the vmalloc address of the framebuffer. For
> > + * framebuffers allocated from contiguous kernel memory, FBINFO_KMEMFB must
> > + * be set, and "fix.smem_start" must be set to the physical address of the
> > + * frame buffer. In both cases, "fix.smem_len" must be set to the framebuffer
> > + * size in bytes.
> > + */
> > +
> > #include <linux/module.h>
> > #include <linux/kernel.h>
> > #include <linux/errno.h>
> > #include <linux/string.h>
> > #include <linux/mm.h>
> > +#include <linux/pfn_t.h>
> > #include <linux/vmalloc.h>
> > #include <linux/delay.h>
> > #include <linux/interrupt.h>
> > @@ -37,7 +66,7 @@ static struct page *fb_deferred_io_get_page(struct fb_info *info, unsigned long
> > else if (info->fix.smem_start)
> > page = pfn_to_page((info->fix.smem_start + offs) >> PAGE_SHIFT);
> >
> > - if (page)
> > + if (page && !(info->flags & FBINFO_KMEMFB))
> > get_page(page);
> >
> > return page;
> > @@ -137,6 +166,15 @@ static vm_fault_t fb_deferred_io_fault(struct vm_fault *vmf)
> >
> > BUG_ON(!info->fbdefio->mapping);
> >
> > + if (info->flags & FBINFO_KMEMFB)
> > + /*
> > + * In this path, the VMA is marked VM_PFNMAP, so mm assumes
> > + * there is no struct page associated with the page. The
> > + * PFN must be directly inserted and the created PTE will be
> > + * marked "special".
> > + */
> > + return vmf_insert_pfn(vmf->vma, vmf->address, page_to_pfn(page));
> > +
> > vmf->page = page;
> > return 0;
> > }
> > @@ -163,13 +201,14 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
> >
> > /*
> > * Adds a page to the dirty list. Call this from struct
> > - * vm_operations_struct.page_mkwrite.
> > + * vm_operations_struct.page_mkwrite or .pfn_mkwrite.
> > */
> > -static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long offset,
> > +static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, struct vm_fault *vmf,
> > struct page *page)
> > {
> > struct fb_deferred_io *fbdefio = info->fbdefio;
> > struct fb_deferred_io_pageref *pageref;
> > + unsigned long offset = vmf->pgoff << PAGE_SHIFT;
> > vm_fault_t ret;
> >
> > /* protect against the workqueue changing the page list */
> > @@ -182,20 +221,34 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
> > }
> >
> > /*
> > - * We want the page to remain locked from ->page_mkwrite until
> > - * the PTE is marked dirty to avoid mapping_wrprotect_range()
> > - * being called before the PTE is updated, which would leave
> > - * the page ignored by defio.
> > - * Do this by locking the page here and informing the caller
> > - * about it with VM_FAULT_LOCKED.
> > + * The PTE must be marked writable before the defio deferred work runs
> > + * again and potentially marks the PTE write-protected. If the order
> > + * should be switched, the PTE would become writable without defio
> > + * tracking the page, leaving the page forever ignored by defio.
> > + *
> > + * For vmalloc() framebuffers, the associated struct page is locked
> > + * before releasing the defio lock. mm will later mark the PTE writaable
> > + * and release the struct page lock. The struct page lock prevents
> > + * the page from being prematurely being marked write-protected.
> > + *
> > + * For FBINFO_KMEMFB framebuffers, mm assumes there is no struct page,
> > + * so the PTE must be marked writable while the defio lock is held.
> > */
> > - lock_page(pageref->page);
> > + if (info->flags & FBINFO_KMEMFB) {
> > + unsigned long pfn = page_to_pfn(pageref->page);
> > +
> > + ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address,
> > + __pfn_to_pfn_t(pfn, PFN_SPECIAL));
>
> Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
> horrible hack.
>
> In another thread, you mention that you use PFN_SPECIAL to bypass the
> check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
The VMA has VM_PFNMAP set, not VM_MIXEDMAP. It seemed like
VM_MIXEDMAP is somewhat of a superset of VM_PFNMAP, but maybe that's
a wrong impression. vm_mixed_ok() does a thorough job of validating the
use of __vm_insert_mixed(), and since what I did was allowed, I thought
perhaps it was OK. Your feedback has set me straight, and that's what I
needed. :-)
But the whole approach is moot with Alistair Popple's patch set that
eliminates pfn_t. Is there an existing mm API that will do mkwrite on a
special PTE in a VM_PFNMAP VMA? I didn't see one, but maybe I missed
it. If there's not one, I'll take a crack at adding it in the next version of my
patch set.
Michael
^ permalink raw reply
* [PATCH v2 1/1] Path string from the host should not be treated as wchar_t
From: yasuenag @ 2025-06-02 23:56 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, ssengar, Yasumasa Suenaga
In-Reply-To: <20250602235612.1542-1-yasuenag@gmail.com>
From: Yasumasa Suenaga <yasuenag@gmail.com>
hv_fcopy_uio_daemon handles file copy request from the host.
(e.g. Copy-VMFile commandlet)
The request has file path and its name, they would be stored as
__u16 arrays in struct hv_start_fcopy. They are casted to wchar_t*
in fcopyd to convert to UTF-8 string. wchar_t is 32bit in Linux
unlike Windows (16bit), so string conversion would be failed and
the user cannot copy file to Linux guest from Host via fcopyd.
fcopyd converts each characters to char if the value is less
than 0x80. Thus we can convert straightly without wcstombs() call,
it means we are no longer to convert to wchar_t.
Length of path depends on PATH_MAX (Linux) and W_MAX_PATH (Windows),
so this change also addes new check to snprintf() call to make
target path.
Signed-off-by: Yasumasa Suenaga <yasuenag@gmail.com>
---
tools/hv/hv_fcopy_uio_daemon.c | 36 +++++++++++++---------------------
1 file changed, 14 insertions(+), 22 deletions(-)
diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
index 0198321d1..85ec21696 100644
--- a/tools/hv/hv_fcopy_uio_daemon.c
+++ b/tools/hv/hv_fcopy_uio_daemon.c
@@ -62,8 +62,10 @@ static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
filesize = 0;
p = path_name;
- snprintf(target_fname, sizeof(target_fname), "%s/%s",
- path_name, file_name);
+ if (snprintf(target_fname, sizeof(target_fname), "%s/%s", path_name, file_name) >= sizeof(target_fname)) {
+ /* target file name is too long */
+ goto done;
+ }
/*
* Check to see if the path is already in place; if not,
@@ -273,6 +275,8 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
while (len < dest_size) {
if (src[len] < 0x80)
dest[len++] = (char)(*src++);
+ else if (src[len] == '0')
+ break;
else
dest[len++] = 'X';
}
@@ -282,27 +286,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
{
- setlocale(LC_ALL, "en_US.utf8");
- size_t file_size, path_size;
- char *file_name, *path_name;
- char *in_file_name = (char *)smsg_in->file_name;
- char *in_path_name = (char *)smsg_in->path_name;
-
- file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
- path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
-
- file_name = (char *)malloc(file_size * sizeof(char));
- path_name = (char *)malloc(path_size * sizeof(char));
-
- if (!file_name || !path_name) {
- free(file_name);
- free(path_name);
- syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
- return HV_E_FAIL;
- }
+ /*
+ * file_name and path_name should have same length with appropriate
+ * member of hv_start_fcopy.
+ */
+ char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
- wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
- wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
+ setlocale(LC_ALL, "en_US.utf8");
+ wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
+ wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
return hv_fcopy_create_file(file_name, path_name, smsg_in->copy_flags);
}
--
2.49.0
^ permalink raw reply related
* [PATCH v2 0/1] Path string from the host should not be treated
From: yasuenag @ 2025-06-02 23:56 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, ssengar, Yasumasa Suenaga
In-Reply-To: <82cbefe0-c9d0-457e-99dd-82842ee64cef@linux.microsoft.com>
From: Yasumasa Suenaga <yasuenag@gmail.com>
Hi,
Easwar, thanks a lot for your comment! I fixed where you pointed.
Let me know if something wrong in this patch - this is my first
contribution to Linux kernel...
Yasumasa Suenaga (1):
Path string from the host should not be treated as wchar_t
tools/hv/hv_fcopy_uio_daemon.c | 36 +++++++++++++---------------------
1 file changed, 14 insertions(+), 22 deletions(-)
--
2.49.0
^ permalink raw reply
* [PATCH v2 0/1] Path string from the host should not be treated
From: yasuenag @ 2025-06-02 23:55 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, ssengar, Yasumasa Suenaga
In-Reply-To: <82cbefe0-c9d0-457e-99dd-82842ee64cef@linux.microsoft.com>
From: Yasumasa Suenaga <yasuenag@gmail.com>
Hi,
Easwar, thanks a lot for your comment! I fixed where you pointed.
Let me know if something wrong in this patch - this is my first
contribution to Linux kernel...
Yasumasa Suenaga (1):
Path string from the host should not be treated as wchar_t
tools/hv/hv_fcopy_uio_daemon.c | 36 +++++++++++++---------------------
1 file changed, 14 insertions(+), 22 deletions(-)
--
2.49.0
^ permalink raw reply
* Re: [PATCH] Path string from the host should not be treated as wchar_t
From: Easwar Hariharan @ 2025-06-02 16:39 UTC (permalink / raw)
To: yasuenag
Cc: kys, haiyangz, wei.liu, decui, eahariha, linux-hyperv,
Saurabh Singh Sengar
In-Reply-To: <20250601134538.3299-1-yasuenag@gmail.com>
On 6/1/2025 6:45 AM, yasuenag@gmail.com wrote:
> From: Yasumasa Suenaga <yasuenag@gmail.com>
>
> hv_fcopy_uio_daemon handles file copy request from the host.
> (e.g. Copy-VMFile commandlet)
> The request has file path and its name, they would be stored as
> __u16 arrays in struct hv_start_fcopy. They are casted to wchar_t*
> in fcopyd to convert to UTF-8 string. wchar_t is 32bit in Linux
> unlike Windows (16bit), so string conversion would be failed and
> the user cannot copy file to Linux guest from Host via fcopyd.
>
> fcopyd converts each characters to char if the value is less
> than 0x80. Thus we can convert straightly without wcstombs() call,
> it means we are no longer to convert to wchar_t.
>
> Length of path depends on PATH_MAX (Linux) and W_MAX_PATH (Windows),
> so this change also addes new check to snprintf() call to make
> target path.
Missing Signed-off-by for developer certificate of origin
> ---
> tools/hv/hv_fcopy_uio_daemon.c | 38 ++++++++++++++--------------------
> 1 file changed, 16 insertions(+), 22 deletions(-)
>
Thanks for the patch! Please look through https://www.kernel.org/doc/html/latest/process/submitting-patches.html
to see hints for the expected subject line, explanation body, and developer's certificate of origin
+CC: Saurabh for review, weird why get_maintainer.pl didn't get him
> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
> index 0198321d1..049d4fd9c 100644
> --- a/tools/hv/hv_fcopy_uio_daemon.c
> +++ b/tools/hv/hv_fcopy_uio_daemon.c
> @@ -58,12 +58,16 @@ static unsigned long long filesize;
> static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
> {
> int error = HV_E_FAIL;
> + int ret_snprintf;
> char *q, *p;
>
> filesize = 0;
> p = path_name;
> - snprintf(target_fname, sizeof(target_fname), "%s/%s",
> - path_name, file_name);
> + ret_snprintf = snprintf(target_fname, sizeof(target_fname), "%s/%s",
> + path_name, file_name);
> + if (ret_snprintf >= sizeof(target_fname))
> + /* target file name is too long */
> + goto done;
The error check can be inlined since we don't use ret_snprintf elsewhere, i.e.
if(snprintf(target_fname, sizeof(target_fname), "%s/%s", path_name, file_name) >= sizeof(target_fname)) {
/* target file name is too long */
goto done;
}
Note also the added braces, even if the extra line is a comment. https://www.kernel.org/doc/html/latest/process/coding-style.html
>
> /*
> * Check to see if the path is already in place; if not,
> @@ -273,6 +277,8 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
> while (len < dest_size) {
> if (src[len] < 0x80)
> dest[len++] = (char)(*src++);
> + else if (src[len] == '0')
> + break;
> else
> dest[len++] = 'X';
> }
> @@ -282,27 +288,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
>
> static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
> {
> - setlocale(LC_ALL, "en_US.utf8");
> - size_t file_size, path_size;
> - char *file_name, *path_name;
> - char *in_file_name = (char *)smsg_in->file_name;
> - char *in_path_name = (char *)smsg_in->path_name;
> -
> - file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
> - path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
> -
> - file_name = (char *)malloc(file_size * sizeof(char));
> - path_name = (char *)malloc(path_size * sizeof(char));
> -
> - if (!file_name || !path_name) {
> - free(file_name);
> - free(path_name);
> - syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
> - return HV_E_FAIL;
> - }
> + /*
> + * file_name and path_name should have same length with appropriate
> + * member of hv_start_fcopy.
> + */
> + char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
>
> - wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
> - wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
> + setlocale(LC_ALL, "en_US.utf8");
> + wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
> + wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
>
> return hv_fcopy_create_file(file_name, path_name, smsg_in->copy_flags);
> }
^ permalink raw reply
* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: David Hildenbrand @ 2025-06-02 9:47 UTC (permalink / raw)
To: mhklinux, simona, deller, haiyangz, kys, wei.liu, decui, akpm
Cc: weh, tzimmermann, hch, dri-devel, linux-fbdev, linux-kernel,
linux-hyperv, linux-mm
In-Reply-To: <20250523161522.409504-4-mhklinux@outlook.com>
On 23.05.25 18:15, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
>
> Current defio code works only for framebuffer memory that is allocated
> with vmalloc(). The code assumes that the underlying page refcount can
> be used by the mm subsystem to manage each framebuffer page's lifecycle,
> including freeing the page if the refcount goes to 0. This approach is
> consistent with vmalloc'ed memory, but not with contiguous kernel memory
> allocated via alloc_pages() or similar. The latter such memory pages
> usually have a refcount of 0 when allocated, and would be incorrectly
> freed page-by-page if used with defio. That free'ing corrupts the memory
> free lists and Linux eventually panics. Simply bumping the refcount after
> allocation doesn’t work because when the framebuffer memory is freed,
> __free_pages() complains about non-zero refcounts.
>
> Commit 37b4837959cb ("video: deferred io with physically contiguous
> memory") from the year 2008 purported to add support for contiguous
> kernel memory framebuffers. The motivating device, sh_mobile_lcdcfb, uses
> dma_alloc_coherent() to allocate framebuffer memory, which is likely to
> use alloc_pages(). It's unclear to me how this commit actually worked at
> the time, unless dma_alloc_coherent() was pulling from a CMA pool instead
> of alloc_pages(). Or perhaps alloc_pages() worked differently or on the
> arm32 architecture on which sh_mobile_lcdcfb is used.
>
> In any case, for x86 and arm64 today, commit 37b4837959cb9 is not
> sufficient to support contiguous kernel memory framebuffers. The problem
> can be seen with the hyperv_fb driver, which may allocate the framebuffer
> memory using vmalloc() or alloc_pages(), depending on the configuration
> of the Hyper-V guest VM (Gen 1 vs. Gen 2) and the size of the framebuffer.
>
> Fix this limitation by adding defio support for contiguous kernel memory
> framebuffers. A driver with a framebuffer allocated from contiguous
> kernel memory must set the FBINFO_KMEMFB flag to indicate such.
>
> Tested with the hyperv_fb driver in both configurations -- with a vmalloc()
> framebuffer and with an alloc_pages() framebuffer on x86. Also verified a
> vmalloc() framebuffer on arm64. Hardware is not available to me to verify
> that the older arm32 devices still work correctly, but the path for
> vmalloc() framebuffers is essentially unchanged.
>
> Even with these changes, defio does not support framebuffers in MMIO
> space, as defio code depends on framebuffer memory pages having
> corresponding 'struct page's.
>
> Fixes: 3a6fb6c4255c ("video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.")
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> ---
> Changes in v3:
> * Moved definition of FBINFO_KMEMFB flag to a separate patch
> preceeding this one in the patch set [Helge Deller]
> Changes in v2:
> * Tweaked code comments regarding framebuffers allocated with
> dma_alloc_coherent() [Christoph Hellwig]
>
> drivers/video/fbdev/core/fb_defio.c | 128 +++++++++++++++++++++++-----
> 1 file changed, 108 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
> index 4fc93f253e06..f8ae91a1c4df 100644
> --- a/drivers/video/fbdev/core/fb_defio.c
> +++ b/drivers/video/fbdev/core/fb_defio.c
> @@ -8,11 +8,40 @@
> * for more details.
> */
>
> +/*
> + * Deferred I/O ("defio") allows framebuffers that are mmap()'ed to user space
> + * to batch user space writes into periodic updates to the underlying
> + * framebuffer hardware or other implementation (such as with a virtualized
> + * framebuffer in a VM). At each batch interval, a callback is invoked in the
> + * framebuffer's kernel driver, and the callback is supplied with a list of
> + * pages that have been modified in the preceding interval. The callback can
> + * use this information to update the framebuffer hardware as necessary. The
> + * batching can improve performance and reduce the overhead of updating the
> + * hardware.
> + *
> + * Defio is supported on framebuffers allocated using vmalloc() and allocated
> + * as contiguous kernel memory using alloc_pages() or kmalloc(). These
> + * memory allocations all have corresponding "struct page"s. Framebuffers
> + * allocated using dma_alloc_coherent() should not be used with defio.
> + * Such allocations should be treated as a black box owned by the DMA
> + * layer, and should not be deconstructed into individual pages as defio
> + * does. Framebuffers in MMIO space are *not* supported because MMIO space
> + * does not have corrresponding "struct page"s.
> + *
> + * For framebuffers allocated using vmalloc(), struct fb_info must have
> + * "screen_buffer" set to the vmalloc address of the framebuffer. For
> + * framebuffers allocated from contiguous kernel memory, FBINFO_KMEMFB must
> + * be set, and "fix.smem_start" must be set to the physical address of the
> + * frame buffer. In both cases, "fix.smem_len" must be set to the framebuffer
> + * size in bytes.
> + */
> +
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/errno.h>
> #include <linux/string.h>
> #include <linux/mm.h>
> +#include <linux/pfn_t.h>
> #include <linux/vmalloc.h>
> #include <linux/delay.h>
> #include <linux/interrupt.h>
> @@ -37,7 +66,7 @@ static struct page *fb_deferred_io_get_page(struct fb_info *info, unsigned long
> else if (info->fix.smem_start)
> page = pfn_to_page((info->fix.smem_start + offs) >> PAGE_SHIFT);
>
> - if (page)
> + if (page && !(info->flags & FBINFO_KMEMFB))
> get_page(page);
>
> return page;
> @@ -137,6 +166,15 @@ static vm_fault_t fb_deferred_io_fault(struct vm_fault *vmf)
>
> BUG_ON(!info->fbdefio->mapping);
>
> + if (info->flags & FBINFO_KMEMFB)
> + /*
> + * In this path, the VMA is marked VM_PFNMAP, so mm assumes
> + * there is no struct page associated with the page. The
> + * PFN must be directly inserted and the created PTE will be
> + * marked "special".
> + */
> + return vmf_insert_pfn(vmf->vma, vmf->address, page_to_pfn(page));
> +
> vmf->page = page;
> return 0;
> }
> @@ -163,13 +201,14 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
>
> /*
> * Adds a page to the dirty list. Call this from struct
> - * vm_operations_struct.page_mkwrite.
> + * vm_operations_struct.page_mkwrite or .pfn_mkwrite.
> */
> -static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long offset,
> +static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, struct vm_fault *vmf,
> struct page *page)
> {
> struct fb_deferred_io *fbdefio = info->fbdefio;
> struct fb_deferred_io_pageref *pageref;
> + unsigned long offset = vmf->pgoff << PAGE_SHIFT;
> vm_fault_t ret;
>
> /* protect against the workqueue changing the page list */
> @@ -182,20 +221,34 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
> }
>
> /*
> - * We want the page to remain locked from ->page_mkwrite until
> - * the PTE is marked dirty to avoid mapping_wrprotect_range()
> - * being called before the PTE is updated, which would leave
> - * the page ignored by defio.
> - * Do this by locking the page here and informing the caller
> - * about it with VM_FAULT_LOCKED.
> + * The PTE must be marked writable before the defio deferred work runs
> + * again and potentially marks the PTE write-protected. If the order
> + * should be switched, the PTE would become writable without defio
> + * tracking the page, leaving the page forever ignored by defio.
> + *
> + * For vmalloc() framebuffers, the associated struct page is locked
> + * before releasing the defio lock. mm will later mark the PTE writaable
> + * and release the struct page lock. The struct page lock prevents
> + * the page from being prematurely being marked write-protected.
> + *
> + * For FBINFO_KMEMFB framebuffers, mm assumes there is no struct page,
> + * so the PTE must be marked writable while the defio lock is held.
> */
> - lock_page(pageref->page);
> + if (info->flags & FBINFO_KMEMFB) {
> + unsigned long pfn = page_to_pfn(pageref->page);
> +
> + ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address,
> + __pfn_to_pfn_t(pfn, PFN_SPECIAL));
Will the VMA have VM_PFNMAP or VM_MIXEDMAP set? PFN_SPECIAL is a
horrible hack.
In another thread, you mention that you use PFN_SPECIAL to bypass the
check in vm_mixed_ok(), so VM_MIXEDMAP is likely not set?
It's all a mess ...
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v3 06/13] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
From: Ricardo Neri @ 2025-06-02 1:31 UTC (permalink / raw)
To: Rob Herring
Cc: Krzysztof Kozlowski, x86, Krzysztof Kozlowski, Conor Dooley,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley, devicetree, Saurabh Sengar, Chris Oo,
linux-hyperv, Kirill A. Shutemov, linux-acpi, linux-kernel,
Ravi V. Shankar, Ricardo Neri
In-Reply-To: <20250529131634.GA2784667-robh@kernel.org>
On Thu, May 29, 2025 at 08:16:34AM -0500, Rob Herring wrote:
> On Sat, May 24, 2025 at 08:56:50AM -0700, Ricardo Neri wrote:
> > On Mon, May 19, 2025 at 10:56:06AM -0700, Ricardo Neri wrote:
> > > On Mon, May 19, 2025 at 10:29:37AM -0500, Rob Herring wrote:
> > > > On Wed, May 14, 2025 at 08:53:38PM -0700, Ricardo Neri wrote:
> > > > > On Wed, May 14, 2025 at 10:42:48AM -0500, Rob Herring wrote:
> > > > > > On Tue, May 13, 2025 at 03:14:56PM -0700, Ricardo Neri wrote:
> > > > > > > On Mon, May 12, 2025 at 10:32:24AM -0500, Rob Herring wrote:
> > > > > > > > On Tue, May 06, 2025 at 08:23:39PM -0700, Ricardo Neri wrote:
> > > > > > > > > On Tue, May 06, 2025 at 09:10:22AM +0200, Krzysztof Kozlowski wrote:
> > > > > > > > > > On Mon, May 05, 2025 at 10:16:10PM GMT, Ricardo Neri wrote:
> > > > > > > > > > > > If this is a device, then compatibles specific to devices. You do not
> > > > > > > > > > > > get different rules than all other bindings... or this does not have to
> > > > > > > > > > > > be binding at all. Why standard reserved-memory does not work for here?
> > > > > > > > > > > >
> > > > > > > > > > > > Why do you need compatible in the first place?
> > > > > > > > > > >
> > > > > > > > > > > Are you suggesting something like this?
> > > > > > > > > > >
> > > > > > > > > > > reserved-memory {
> > > > > > > > > > > # address-cells = <2>;
> > > > > > > > > > > # size-cells = <1>;
> > > > > > > > > > >
> > > > > > > > > > > wakeup_mailbox: wakeupmb@fff000 {
> > > > > > > > > > > reg = < 0x0 0xfff000 0x1000>
> > > > > > > > > > > }
> > > > > > > > > > >
> > > > > > > > > > > and then reference to the reserved memory using the wakeup_mailbox
> > > > > > > > > > > phandle?
> > > > > > > > > >
> > > > > > > > > > Yes just like every other, typical reserved memory block.
> > > > > > > > >
> > > > > > > > > Thanks! I will take this approach and drop this patch.
> > > > > > > >
> > > > > > > > If there is nothing else to this other than the reserved region, then
> > > > > > > > don't do this. Keep it like you had. There's no need for 2 nodes.
> > > > > > >
> > > > > > > Thank you for your feedback!
> > > > > > >
> > > > > > > I was planning to use one reserved-memory node and inside of it a child
> > > > > > > node to with a `reg` property to specify the location and size of the
> > > > > > > mailbox. I would reference to that subnode from the kernel code.
> > > > > > >
> > > > > > > IIUC, the reserved-memory node is only the container and the actual memory
> > > > > > > regions are expressed as child nodes.
> > > > > > >
> > > > > > > I had it like that before, but with a `compatible` property that I did not
> > > > > > > need.
> > > > > > >
> > > > > > > Am I missing anything?
> > > > > >
> > > > > > Without a compatible, how do you identify which reserved region is the
> > > > > > wakeup mailbox?
> > > > >
> > > > > I thought using a phandle to the wakeup_mailbox. Then I realized that the
> > > > > device nodes using the mailbox would be CPUs. They would need a `memory-
> > > > > region` property. This does not look right to me.
> > > >
> > > > That doesn't really make sense unless it's a memory region per CPU.
> > >
> > > Agreed.
> > >
> > > >
> > > >
> > > > > > Before you say node name, those are supposed to be
> > > > > > generic though we failed to enforce anything for /reserved-memory child
> > > > > > nodes.
> > > > >
> > > > > I see. Thanks for preventing me from doing this.
> > > > >
> > > > > Then the `compatible` property seems the way to go after all.
> > > > >
> > > > > This what motivated this patch in the first place. On further analysis,
> > > > > IIUC, defining bindings and schema is not needed, IMO, since the mailbox
> > > > > is already defined in the ACPI spec. No need to redefine.
> > > >
> > > > You lost me...
> > > >
> > > > You don't need to redefine the layout of the memory region as that's
> > > > defined already somewhere,
> > >
> > > Great!
> > >
> > > > but you do need to define where it is for DT.
> > > > And for that, you need a compatible. Do you know where it is in this
> > > > case?
> > >
> > > The compatible is not defined anywhere yet. Is a DT schema needed to
> > > document it? If yes, I am usure what to put in the description. We tried
> > > to not redefine the mailbox and refer to the ACPI spec. That was a NAK
> > > from Krzysztof [1].
> > >
> > > [1]. https://lore.kernel.org/r/624e1985-7dd2-4abe-a918-78cb43556967@kernel.org
> >
> > In summary, documenting the `compatible` property for the mailbox is
> > necessary. There is no need to redefine the malbox on a schema but
> > referring to the ACPI spec is not acceptable.
>
> There's the whole "DT bindings in ACPI systems" where ACPI tables
> contain compatibles and DT properties which I think is what
> Krzysztof was objecting to (and I do too). But this is a DT based system
> that implements a mailbox region defined in an ACPI spec. That is
> perfectly fine to refer to.
That is correct. It is a DT-based system.
Great! I will refer to the ACPI spec in my schema in my next version.
>
> >
> > What about referring in the schema to the Intel TDX Virtual Firmware Design
> > Guide[2]? It describes how firmware should implement the mailbox the section
> > 4.3.5.
> >
> > A mailbox with compatible = "intel,wakeup-mailbox" is implemented after the
> > guide that Intel published.
>
> Use whatever you think best describes the programming model of the
> region.
Understood.
Thanks and BR,
Ricardo
^ permalink raw reply
* Re: [PATCH v4 0/5] Allow dyn MSI-X vector allocation of MANA
From: Zhu Yanjun @ 2025-06-01 14:53 UTC (permalink / raw)
To: Shradha Gupta
Cc: linux-hyperv, linux-pci, linux-kernel, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy�~Dski,
Lorenzo Pieralisi, Dexuan Cui, Wei Liu, Haiyang Zhang,
K. Y. Srinivasan, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Konstantin Taranov, Simon Horman,
Leon Romanovsky, Maxim Levitsky, Erni Sri Satya Vennela,
Peter Zijlstra, netdev, linux-rdma, Paul Rosswurm, Shradha Gupta
In-Reply-To: <1748361453-25096-1-git-send-email-shradhagupta@linux.microsoft.com>
在 2025/5/27 17:57, Shradha Gupta 写道:
> In this patchset we want to enable the MANA driver to be able to
> allocate MSI-X vectors in PCI dynamically.
>
> The first patch exports pci_msix_prepare_desc() in PCI to be able to
> correctly prepare descriptors for dynamically added MSI-X vectors.
>
> The second patch adds the support of dynamic vector allocation in
> pci-hyperv PCI controller by enabling the MSI_FLAG_PCI_MSIX_ALLOC_DYN
> flag and using the pci_msix_prepare_desc() exported in first patch.
>
> The third patch adds a detailed description of the irq_setup(), to
> help understand the function design better.
>
> The fourth patch is a preparation patch for mana changes to support
> dynamic IRQ allocation. It contains changes in irq_setup() to allow
> skipping first sibling CPU sets, in case certain IRQs are already
> affinitized to them.
>
> The fifth patch has the changes in MANA driver to be able to allocate
> MSI-X vectors dynamically. If the support does not exist it defaults to
> older behavior.
> ---
> Change in v4
> * add a patch describing the functionality of irq_setup() through a
> comment
> * In irq_setup(), avoid using a label next_cpumask:
> * modify the changes in MANA patch about restructuring the error
> handling path in mana_gd_setup_dyn_irqs()
> * modify the mana_gd_setup_irqs() to simplify handling around
> start_irq_index
> * add warning if an invalid gic is returned
> * place the xa_destroy() cleanup in mana_gd_remove
> ---
> Changes in v3
> * split the 3rd patch into preparation patch around irq_setup() and
> changes in mana driver to allow dynamic IRQ allocation
> * Add arm64 support for dynamic MSI-X allocation in pci_hyperv
> controller
> ---
> Changes in v2
> * split the first patch into two(exporting the preapre_desc
> func and using the function and flag in pci-hyperv)
> * replace 'pci vectors' by 'MSI-X vectors'
> * Change the cover letter description to align with changes made
> ---
>
> Shradha Gupta (5):
> PCI/MSI: Export pci_msix_prepare_desc() for dynamic MSI-X allocations
> PCI: hv: Allow dynamic MSI-X vector allocation
> net: mana: explain irq_setup() algorithm
> net: mana: Allow irq_setup() to skip cpus for affinity
> net: mana: Allocate MSI-X vectors dynamically
In this patchset, base-commit seems missing.
Please see this link:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.15#n868
"
When you open ``outgoing/0000-cover-letter.patch`` for editing, you will
notice that it will have the ``base-commit:`` trailer at the very
bottom, which provides the reviewer and the CI tools enough information
to properly perform ``git am`` without worrying about conflicts::
"
When creating patches:
"
git format-patch --base=main origin/main
"
This will include a base-commit: line in each patch file:
"
base-commit: abcdef1234567890...
"
This is useful when submitting patches to mailing lists or other tooling.
Please follow the submitting-patches.rst to add base-commit.
Best Regards,
Zhu Yanjun
>
> .../net/ethernet/microsoft/mana/gdma_main.c | 356 ++++++++++++++----
> drivers/pci/controller/pci-hyperv.c | 5 +-
> drivers/pci/msi/irqdomain.c | 5 +-
> include/linux/msi.h | 2 +
> include/net/mana/gdma.h | 8 +-
> 5 files changed, 293 insertions(+), 83 deletions(-)
>
^ permalink raw reply
* [PATCH] Path string from the host should not be treated as wchar_t
From: yasuenag @ 2025-06-01 13:45 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, Yasumasa Suenaga
From: Yasumasa Suenaga <yasuenag@gmail.com>
hv_fcopy_uio_daemon handles file copy request from the host.
(e.g. Copy-VMFile commandlet)
The request has file path and its name, they would be stored as
__u16 arrays in struct hv_start_fcopy. They are casted to wchar_t*
in fcopyd to convert to UTF-8 string. wchar_t is 32bit in Linux
unlike Windows (16bit), so string conversion would be failed and
the user cannot copy file to Linux guest from Host via fcopyd.
fcopyd converts each characters to char if the value is less
than 0x80. Thus we can convert straightly without wcstombs() call,
it means we are no longer to convert to wchar_t.
Length of path depends on PATH_MAX (Linux) and W_MAX_PATH (Windows),
so this change also addes new check to snprintf() call to make
target path.
---
tools/hv/hv_fcopy_uio_daemon.c | 38 ++++++++++++++--------------------
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
index 0198321d1..049d4fd9c 100644
--- a/tools/hv/hv_fcopy_uio_daemon.c
+++ b/tools/hv/hv_fcopy_uio_daemon.c
@@ -58,12 +58,16 @@ static unsigned long long filesize;
static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
{
int error = HV_E_FAIL;
+ int ret_snprintf;
char *q, *p;
filesize = 0;
p = path_name;
- snprintf(target_fname, sizeof(target_fname), "%s/%s",
- path_name, file_name);
+ ret_snprintf = snprintf(target_fname, sizeof(target_fname), "%s/%s",
+ path_name, file_name);
+ if (ret_snprintf >= sizeof(target_fname))
+ /* target file name is too long */
+ goto done;
/*
* Check to see if the path is already in place; if not,
@@ -273,6 +277,8 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
while (len < dest_size) {
if (src[len] < 0x80)
dest[len++] = (char)(*src++);
+ else if (src[len] == '0')
+ break;
else
dest[len++] = 'X';
}
@@ -282,27 +288,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
{
- setlocale(LC_ALL, "en_US.utf8");
- size_t file_size, path_size;
- char *file_name, *path_name;
- char *in_file_name = (char *)smsg_in->file_name;
- char *in_path_name = (char *)smsg_in->path_name;
-
- file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
- path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
-
- file_name = (char *)malloc(file_size * sizeof(char));
- path_name = (char *)malloc(path_size * sizeof(char));
-
- if (!file_name || !path_name) {
- free(file_name);
- free(path_name);
- syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
- return HV_E_FAIL;
- }
+ /*
+ * file_name and path_name should have same length with appropriate
+ * member of hv_start_fcopy.
+ */
+ char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
- wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
- wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
+ setlocale(LC_ALL, "en_US.utf8");
+ wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
+ wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
return hv_fcopy_create_file(file_name, path_name, smsg_in->copy_flags);
}
--
2.49.0
^ permalink raw reply related
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