* RE: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Michael Kelley @ 2025-06-04 21:43 UTC (permalink / raw)
To: Simona Vetter, Thomas Zimmermann
Cc: 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, 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: <aEBcCjMWZJgbsRas@phenom.ffwll.local>
From: Simona Vetter <simona.vetter@ffwll.ch> Sent: Wednesday, June 4, 2025 7:46 AM
>
> On Wed, Jun 04, 2025 at 10:12:45AM +0200, Thomas Zimmermann wrote:
> > Hi
> >
> > Am 03.06.25 um 19:50 schrieb Michael Kelley:
> > > 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:
> > > > [...]
> > > > 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.
> >
> > We (DRM upstream) recently considered moving some fbdev drivers to
> > drivers/staging or marking them with !DRM if a DRM driver is available.
> > Hyverv_fb would be a candidate.
> >
> > At least at SUSE, we ship hypervdrm instead of hyperv_fb. This works well on
> > the various generations of the hyperv system. Much of our userspace would
> > not be able to use hyperv_fb anyway.
Good to know. Red Hat has made the switch as well. The Ubuntu images
in Azure have both hyperv_fb and hyperv_drm. I don't know what other
distros have done.
>
> Yeah investing into fbdev drivers, especially when some mm surgery seems
> needed, does not sound like a good idea to me overall.
>
> > > 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.
> >
> > The panic output is a feature that we recently added to the kernel. It
> > allows a DRM driver to display a final error message in the case of a kernel
> > panic (think of blue screens on Windows). Drivers require a minimum of
> > support to make it work. That's what the hypervdrm patches were about.
>
> I'm also happy to help with any other issues and shortfalls of drm vs
> fbdev. There are some, but I thought it was mostly around some of the low
> bit color formats that really old devices want, and not anything that
> hyperv would need.
You've set me up perfectly to raise an issue. :-) I'm still relatively new
to the hyperv_drm driver and DRM in general, compared with hyperv_fb.
One capability in fbdev is deferred I/O, which is what this entire patch
series is about. The hyperv_drm driver doesn't currently use anything
similar to deferred I/O like hyperv_fb. I don't know if that's because
hyperv_drm doesn't make use of what DRM has to offer, or if DRM doesn't
have a deferred I/O framework like fbdev. Do you know what the situation
is? Or could you point me to an example of doing deferred I/O with DRM
that hyperv_drm should be following?
I ran a quick performance test comparing hyperv_drm with hyperv_fb.
The test does "cat" of a big text file in the Hyper-V graphics console. The
file has 1024 * 1024 lines, each with 64 characters, so total file size is
64 MiB.
With hyperv_fb the test completes in 24 seconds elapsed time, with
24 seconds of system CPU time. With hyperv_drm, it takes 34 seconds
elapsed time, but with about the same 24 seconds of system CPU time.
Overall this difference isn't huge, and probably isn't that noticeable
when doing human-scale work (i.e., 'dmesg' outputting several
hundred lines in 0.19 seconds vs. my test doing 1M lines) on the Hyper-V
graphics console. To me, the console doesn't feel slow with hyperv_drm
compared to hyperv_fb, which is good.
Nonetheless, there's an underlying issue. A main cause of the difference
is the number of messages to Hyper-V to update dirty regions. With
hyperv_fb using deferred I/O, the messages are limited 20/second, so
the total number of messages to Hyper-V is about 480. But hyperv_drm
appears to send 3 messages to Hyper-V for each line of output, or a total of
about 3,000,000 messages (~90K/second). That's a lot of additional load
on the Hyper-V host, and it adds the 10 seconds of additional elapsed
time seen in the guest. There also this ugly output in dmesg because the
ring buffer for sending messages to the Hyper-V host gets full -- Hyper-V
doesn't always keep up, at least not on my local laptop where I'm
testing:
[12574.327615] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12574.327684] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12574.327760] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12574.327841] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12597.016128] hyperv_sendpacket: 6211 callbacks suppressed
[12597.016133] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12597.016172] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12597.016220] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
[12597.016267] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] *ERROR* Unable to send packet via vmbus; error -11
hyperv_drm could be fixed to not output the ugly messages, but there's
still the underlying issue of overrunning the ring buffer, and excessively
hammering on the host. If we could get hyperv_drm doing deferred I/O, I
would feel much better about going full-on with deprecating hyperv_fb.
Michael
^ permalink raw reply
* Re: [PATCH v3 1/7] Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
From: Easwar Hariharan @ 2025-06-04 17:41 UTC (permalink / raw)
To: Michael Kelley
Cc: eahariha, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, lpieralisi@kernel.org, kw@linux.com,
manivannan.sadhasivam@linaro.org, robh@kernel.org,
bhelgaas@google.com, arnd@arndb.de, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-pci@vger.kernel.org, linux-arch@vger.kernel.org
In-Reply-To: <1643d6a8-7d4f-4d6e-aeab-f43963644a1f@linux.microsoft.com>
Hi Michael,
On 4/21/2025 4:27 PM, Easwar Hariharan wrote:
> On 4/21/2025 2:24 PM, Michael Kelley wrote:
>> From: Easwar Hariharan <eahariha@linux.microsoft.com> Sent: Monday, April 21, 2025 1:41 PM
>>>>
>
> <snip>
>
>>>>
>>>
>>> This is very cool, thanks for taking the time! I think the function naming
>>> could be more intuitive, e.g. hv_setup_*_args(). I'd not block it for that reason,
>>> but would be super happy if you would update it. What do you think?
>>>
>>
>> I'm not particularly enamored with my naming scheme, but it was the
>> best I could come up with. My criteria were:
>>
>> * Keep the length reasonably short to not make line length problems
>> any worse
>> * Distinguish the input args only, input & output args, and array versions
>
> I think the in/inout/array scheme you have does this nicely
>
>> * Use the standard "hv_" prefix for Hyper-V related code
>>
>> Using "setup" instead of "hvcall" seems like an improvement to me, and
>> it is 1 character shorter. The "hv" prefix would be there, but they wouldn't
>> refer specifically to hypercalls. I would not add "_args" on the end because
>> that's another 5 characters in length. So we would have:
>>
>> * hv_setup_in()
>> * hv_setup_inout()
>> * hv_setup_in_array()
>> * hv_setup_inout_array()
>> * hv_setup_in_batch_size() [??]
>>
>> Or maybe, something like this, or similar, which picks up the "args" string,
>> but not "setup":
>>
>> * hv_hcargs_in()
>> * hv_hcargs_inout()
>> * hv_hcargs_in_array()
>> * hv_hcargs_inout_array()
>> * hv_hcargs_in_batch_size() [??]
>>
>> I'm very open to any other ideas because I'm not particularly
>> happy with the hv_hvcall_* approach.
>
> Between the two presented here, I prefer option 1, with the "setup" verb because it tells you
> inline what the function will do. I agree that the "args" is unnecessary because most
> hypercall args are named hv_{input, output}_* and are clearly arguments to hv_do_hypercall()
> and friends.
>
> Since hv_setup*() will normally be followed shortly after by hv_do_hypercall(), I don't
> see a problem with not referring specifically to hypercalls, it should be clear in context.
>
> For hv_hvcall_in_batch_size(), I think it serves a fundamentally different function than the
> other wrappers and doesn't need to follow the "setup" pattern. Instead it could be named
> hv_get_input_batch_size() for the same length and similarly tell you its purpose inline.
>
> I am continuing to review the rest of the series, sorry for the delay, and thank you for your
> patience!
>
Sorry this took so long, commercial work took up all of my focus the past few weeks. The rest
of the patches look good to me. If you could follow up with a v4 for the function naming,
Wei can pick this up for the 6.17 merge window.
Thanks,
Easwar (he/him)
^ permalink raw reply
* Re: [PATCH v4 03/10] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
From: Ricardo Neri @ 2025-06-04 15:17 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: Michael Kelley, linux-acpi, Wei Liu, Conor Dooley, Yunhong Jiang,
K. Y. Srinivasan, Chris Oo, Ricardo Neri, linux-hyperv,
Haiyang Zhang, Krzysztof Kozlowski, linux-kernel, Ravi V. Shankar,
Rafael J. Wysocki, Kirill A. Shutemov, x86, Saurabh Sengar,
Dexuan Cui, devicetree
In-Reply-To: <174900070284.2624702.4580450009482590306.robh@kernel.org>
On Tue, Jun 03, 2025 at 08:31:42PM -0500, Rob Herring (Arm) wrote:
>
> On Tue, 03 Jun 2025 17:15:15 -0700, Ricardo Neri wrote:
> > Add DeviceTree bindings to enumerate the wakeup mailbox used in platform
> > firmware for Intel processors.
> >
> > x86 platforms commonly boot secondary CPUs using an INIT assert, de-assert
> > followed by Start-Up IPI messages. The wakeup mailbox can be used when this
> > mechanism is unavailable.
> >
> > The wakeup mailbox offers more control to the operating system to boot
> > secondary CPUs than a spin-table. It allows the reuse of same wakeup vector
> > for all CPUs while maintaining control over which CPUs to boot and when.
> > While it is possible to achieve the same level of control using a spin-
> > table, it would require to specify a separate `cpu-release-addr` for each
> > secondary CPU.
> >
> > The operation and structure of the mailbox is described in the
> > Multiprocessor Wakeup Structure defined in the ACPI specification. Note
> > that this structure does not specify how to publish the mailbox to the
> > operating system (ACPI-based platform firmware uses a separate table). No
> > ACPI table is needed in DeviceTree-based firmware to enumerate the mailbox.
> >
> > Add a `compatible` property that the operating system can use to discover
> > the mailbox. Nodes wanting to refer to the reserved memory usually define a
> > `memory-region` property. /cpus/cpu* nodes would want to refer to the
> > mailbox, but they do not have such property defined in the DeviceTree
> > specification. Moreover, it would imply that there is a memory region per
> > CPU.
> >
> > Co-developed-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> > ---
> > Changes since v3:
> > - Removed redefinitions of the mailbox and instead referred to ACPI
> > specification as per discussion on LKML.
> > - Clarified that DeviceTree-based firmware do not require the use of
> > ACPI tables to enumerate the mailbox. (Rob)
> > - Described the need of using a `compatible` property.
> > - Dropped the `alignment` property. (Krzysztof, Rafael)
> > - Used a real address for the mailbox node. (Krzysztof)
> >
> > Changes since v2:
> > - Implemented the mailbox as a reserved-memory node. Add to it a
> > `compatible` property. (Krzysztof)
> > - Explained the relationship between the mailbox and the `enable-mehod`
> > property of the CPU nodes.
> > - Expanded the documentation of the binding.
> >
> > Changes since v1:
> > - Added more details to the description of the binding.
> > - Added requirement a new requirement for cpu@N nodes to add an
> > `enable-method`.
> > ---
> > .../reserved-memory/intel,wakeup-mailbox.yaml | 48 ++++++++++++++++++++++
> > 1 file changed, 48 insertions(+)
> >
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml:20:111: [warning] line too long (113 > 110 characters) (line-length)
I did see this warning. I also see that none of the existing schema with
links break them into multiple lines.
Now I see that the yamllint configuration has allow-non-breakable-words: true
I will put the link in separate line.
^ permalink raw reply
* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Simona Vetter @ 2025-06-04 14:45 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: 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, 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: <9a93813c-4d7c-45ef-b5a2-0ad37e7a078a@suse.de>
On Wed, Jun 04, 2025 at 10:12:45AM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 03.06.25 um 19:50 schrieb Michael Kelley:
> > 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.
>
> We (DRM upstream) recently considered moving some fbdev drivers to
> drivers/staging or marking them with !DRM if a DRM driver is available.
> Hyverv_fb would be a candidate.
>
> At least at SUSE, we ship hypervdrm instead of hyperv_fb. This works well on
> the various generations of the hyperv system. Much of our userspace would
> not be able to use hyperv_fb anyway.
Yeah investing into fbdev drivers, especially when some mm surgery seems
needed, does not sound like a good idea to me overall.
> > 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.
>
> The panic output is a feature that we recently added to the kernel. It
> allows a DRM driver to display a final error message in the case of a kernel
> panic (think of blue screens on Windows). Drivers require a minimum of
> support to make it work. That's what the hypervdrm patches were about.
I'm also happy to help with any other issues and shortfalls of drm vs
fbdev. There are some, but I thought it was mostly around some of the low
bit color formats that really old devices want, and not anything that
hyperv would need.
Cheers, Sima
>
> Best regards
> Thomas
>
> >
> > 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
>
> --
> --
> 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)
>
--
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* Re: [PATCH hyperv-next v3 13/15] Drivers: hv: Support confidential VMBus channels
From: ALOK TIWARI @ 2025-06-04 14:15 UTC (permalink / raw)
To: Roman Kisel, arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa,
kys, mingo, mhklinux, tglx, wei.liu, linux-arch, linux-doc,
linux-hyperv, linux-kernel, x86
Cc: apais, benhill, bperkins, sunilmut
In-Reply-To: <20250604004341.7194-14-romank@linux.microsoft.com>
On 04-06-2025 06:13, Roman Kisel wrote:
> To run a confidential VMBus channels, one has to initialize the
> co_ring_buffers and co_external_memory fields of the channel
> structure.
>
> Advertise support upon negoatiating the version and compute
> values for those fields and initialize them.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---
> drivers/hv/channel_mgmt.c | 19 +++++++++++++++++++
> drivers/hv/connection.c | 3 +++
> 2 files changed, 22 insertions(+)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index ca2fe10c110a..33bc29e826bd 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -1021,6 +1021,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
> struct vmbus_channel_offer_channel *offer;
> struct vmbus_channel *oldchannel, *newchannel;
> size_t offer_sz;
> + bool co_ring_buffer, co_external_memory;
>
> offer = (struct vmbus_channel_offer_channel *)hdr;
>
> @@ -1033,6 +1034,22 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
> return;
> }
>
> + co_ring_buffer = is_co_ring_buffer(offer);
> + if (co_ring_buffer) {
> + if (vmbus_proto_version < VERSION_WIN10_V6_0 || !vmbus_is_confidential()) {
> + atomic_dec(&vmbus_connection.offer_in_progress);
> + return;
> + }
> + }
> +
> + co_external_memory = is_co_external_memory(offer);
> + if (is_co_external_memory(offer)) {
Redundant call for is_co_external_memory()
if(co_external_memory)
> + if (vmbus_proto_version < VERSION_WIN10_V6_0 || !vmbus_is_confidential()) {
> + atomic_dec(&vmbus_connection.offer_in_progress);
> + return;
> + }
> + }
> +
> oldchannel = find_primary_channel_by_offer(offer);
>
> if (oldchannel != NULL) {
> @@ -1111,6 +1128,8 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
> pr_err("Unable to allocate channel object\n");
> return;
> }
> + newchannel->co_ring_buffer = co_ring_buffer;
> + newchannel->co_external_memory = co_external_memory;
>
> vmbus_setup_channel_state(newchannel, offer);
>
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index be490c598785..eeb472019d69 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -105,6 +105,9 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
> vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID;
> }
>
> + if (vmbus_is_confidential() && version >= VERSION_WIN10_V6_0)
> + msg->feature_flags = VMBUS_FEATURE_FLAG_CONFIDENTIAL_CHANNELS;
> +
> /*
> * shared_gpa_boundary is zero in non-SNP VMs, so it's safe to always
> * bitwise OR it
Thanks,
Alok
^ permalink raw reply
* Re: [PATCH hyperv-next v3 07/15] Drivers: hv: Post messages via the confidential VMBus if available
From: ALOK TIWARI @ 2025-06-04 13:48 UTC (permalink / raw)
To: Roman Kisel, arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa,
kys, mingo, mhklinux, tglx, wei.liu, linux-arch, linux-doc,
linux-hyperv, linux-kernel, x86
Cc: apais, benhill, bperkins, sunilmut
In-Reply-To: <20250604004341.7194-8-romank@linux.microsoft.com>
On 04-06-2025 06:13, Roman Kisel wrote:
> - if (ms_hyperv.paravisor_present) {
> + if (ms_hyperv.paravisor_present && !vmbus_is_confidential()) {
it is, not vmbus_is_confidential() ?
> if (hv_isolation_type_tdx())
> status = hv_tdx_hypercall(HVCALL_POST_MESSAGE,
> virt_to_phys(aligned_msg), 0);
Thanks,
Alok
^ permalink raw reply
* Re: [PATCH hyperv-next v3 04/15] arch/x86: mshyperv: Trap on access for some synthetic MSRs
From: ALOK TIWARI @ 2025-06-04 13:38 UTC (permalink / raw)
To: Roman Kisel, arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa,
kys, mingo, mhklinux, tglx, wei.liu, linux-arch, linux-doc,
linux-hyperv, linux-kernel, x86
Cc: apais, benhill, bperkins, sunilmut
In-Reply-To: <20250604004341.7194-5-romank@linux.microsoft.com>
On 04-06-2025 06:13, Roman Kisel wrote:
> + /*
> + * Write proxy bit in the case of non-confidential VMBus.
> + * Using wrmsl instruction so the following goes to the paravisor.
wrmsl -> wrmsrl
> + */
> + u32 proxy = vmbus_is_confidential() ? 0 : 1;
> +
> + value |= (proxy << 20);
> + native_wrmsrl(reg, value);
> + }
> } else {
Thanks,
Alok
^ permalink raw reply
* Re: [PATCH hyperv-next v3 03/15] arch: hyperv: Get/set SynIC synth.registers via paravisor
From: ALOK TIWARI @ 2025-06-04 13:27 UTC (permalink / raw)
To: Roman Kisel, arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa,
kys, mingo, mhklinux, tglx, wei.liu, linux-arch, linux-doc,
linux-hyperv, linux-kernel, x86
Cc: apais, benhill, bperkins, sunilmut
In-Reply-To: <20250604004341.7194-4-romank@linux.microsoft.com>
On 04-06-2025 06:13, Roman Kisel wrote:
> The confidential VMBus is built on the guest talking to the
> paravisor only.
>
> Provide functions that allow manipulating the SynIC registers
> via paravisor. No functional changes.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> ---
> arch/x86/kernel/cpu/mshyperv.c | 44 ++++++++++++++++++++++++++++++++++
> drivers/hv/hv_common.c | 13 ++++++++++
> include/asm-generic/mshyperv.h | 2 ++
> 3 files changed, 59 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 3e2533954675..83a85d94bcb3 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -89,6 +89,50 @@ void hv_set_non_nested_msr(unsigned int reg, u64 value)
> }
> EXPORT_SYMBOL_GPL(hv_set_non_nested_msr);
>
> +/*
> + * Attempt to get the SynIC register value from the paravisor.
> + *
> + * Not all paravisors support reading SynIC registers, so this function
> + * may fail. The register for the SynIC of the running CPU is accessed.
> + *
> + * Writes the SynIC register value into the memory pointed by val,
> + * and ~0ULL is on failure.
> + *
> + * Returns -ENODEV if the MSR is not a SynIC register, or another error
> + * code if getting the MSR fails (meaning the paravisor doesn't support
> + * relaying VMBus comminucations).
comminucations -> communications
> + */
> +int hv_para_get_synic_register(unsigned int reg, u64 *val)
> +{
> + u64 reg_val = ~0ULL;
> + int err = -ENODEV;
> +
> + if (hv_is_synic_msr(reg))
> + reg_val = native_read_msr_safe(reg, &err);
> + *val = reg_val;
> +
> + return err;
> +}
> +
> +/*
> + * Attempt to set the SynIC register value with the paravisor.
> + *
> + * Not all paravisors support reading SynIC registers, so this function
> + * may fail. The register for the SynIC of the running CPU is accessed.
you are writing, not reading. ?
-> "Not all paravisors support writing SynIC registers"
> + *
> + * Sets the register to the value supplied.
> + *
> + * Returns: -ENODEV if the MSR is not a SynIC register, or another error
> + * code if writing to the MSR fails (meaning the paravisor doesn't support
> + * relaying VMBus comminucations).
> + */
> +int hv_para_set_synic_register(unsigned int reg, u64 val)
> +{
> + if (!hv_is_synic_msr(reg))
> + return -ENODEV;
> + return wrmsrl_safe(reg, val);
> +}
Thanks,
Alok
^ permalink raw reply
* Re: [PATCH hyperv-next v3 01/15] Documentation: hyperv: Confidential VMBus
From: ALOK TIWARI @ 2025-06-04 13:20 UTC (permalink / raw)
To: Roman Kisel, arnd, bp, corbet, dave.hansen, decui, haiyangz, hpa,
kys, mingo, mhklinux, tglx, wei.liu, linux-arch, linux-doc,
linux-hyperv, linux-kernel, x86
Cc: apais, benhill, bperkins, sunilmut
In-Reply-To: <20250604004341.7194-2-romank@linux.microsoft.com>
On 04-06-2025 06:13, Roman Kisel wrote:
> +Confidential VMBus is an extension of Confidential Computing (CoCo) VMs
> +(a.k.a. "Isolated" VMs in Hyper-V terminology). Without Confidential VMBus,
> +guest VMBus device drivers (the "VSC"s in VMBus terminology) communicate
> +with VMBus servers (the VSPs) running on the Hyper-V host. The
> +communication must be through memory that has been decrypted so the
> +host can access it. With Confidential VMBus, one or more of the VSPs reside
> +in the trusted paravisor layer in the guest VM. Since the paravisor layer also
> +operates in encrypted memory, the memory used for communication with
> +such VSPs does not need to be decrypted and thereby exposed to the
> +Hyper-V host. The paravisor is responsible for communicating securely
> +with the Hyper-V host as necessary. In some cases (e.g. time synchonization,
Typo synchonization -> synchronization
> +key-value pairs exchange) the unencrypted data doesn't need to be communicated
> +with the host at all, and a conventional VMBus connection suffices.
Thanks,
Alok
^ permalink raw reply
* Re: [PATCH v3 1/1] hv_fcopy_uio_daemon: Fix file copy failure between Windows host and Linux guest
From: Yasumasa Suenaga @ 2025-06-04 13:12 UTC (permalink / raw)
To: Naman Jain, eahariha; +Cc: kys, haiyangz, wei.liu, decui, linux-hyperv, ssengar
In-Reply-To: <fe8524f6-3f5f-4266-b9e5-f200ba04e1a9@linux.microsoft.com>
Hi Naman,
Thanks a lot for your help!
I will fix for your comment, and send as v4 patch next week :)
> Please help me understand if this is correct. The actual problem you are highlighting lies here when sizeof(wchat_t) is more than 16, i.e. 32?
Yes, so we should not cast array of __u16 to wchar_t*.
I confirmed it on Linux as following:
```
[yasuenag@fc42 sizeof]$ cat test.c
#include <stdio.h>
#include <stddef.h>
int main(){
printf("%ld\n", sizeof(wchar_t));
return 0;
}
[yasuenag@fc42 sizeof]$ rpm -q gcc
gcc-15.1.1-2.fc42.x86_64
[yasuenag@fc42 sizeof]$ gcc test.c
[yasuenag@fc42 sizeof]$ ./a.out
4
```
In Windows, it returns 2:
```
PS C:\test\sizeof> cl.exe .\test.c
Microsoft(R) C/C++ Optimizing Compiler Version 19.44.35208 for x86
Copyright (C) Microsoft Corporation. All rights reserved.
test.c
Microsoft (R) Incremental Linker Version 14.44.35208.0
Copyright (C) Microsoft Corporation. All rights reserved.
/out:test.exe
test.obj
PS C:\test\sizeof> .\test.exe
2
```
Thanks,
Yasumasa
On 2025/06/04 21:36, Naman Jain wrote:
>
>
> On 6/4/2025 2:40 PM, Yasumasa Suenaga wrote:
>> Hi Naman,
>> Sorry for bothering you, but I have some questions:
>>
>>
>
> Hi,
> Happy to help. I have tested your patch on my setup, and it did not break anything for me. Please find response and additional questions inline.
>
>
>>> 1. Create a new patch file for every new version and then send it.
>>> Currently it seems you are manually editing the same patch file in the
>>> subject and sending it, so each patch version is showing up in the same
>>> thread.
>>
>> I've committed changes with "git amend" and created formatted patch with "git format-patch",
>> then I sent all of *.patch files via "git send-email".
>> Do you mean I should commit to fix commits reviewers and send cover letter and incremental
>> patches only? I couldn't find about it from the guide.
>> I checked linux-hyperv list, all of patches have been sent in each version AFAICS.
>>
>
> * make changes
> * git add, git commit, git format-patch, git send-email <path to v1 patch)
> * make changes for v2
> * git add, git commit --amend, git format-patch -v2, git send-email <path to v2 patch *only*>
> and so on.
>
> I follow above practice, and the patches go in new threads. I am not sure why they are landing in same thread for you. Anyhow, its not a big thing.
>
>>
>>> 3. Keep a minimum of 1-2 weeks gap between successive patch versions to
>>> give time to people to review your changes.
>>
>> Does it include changes of commit message too?
>
> Other reviewers may be reviewing older versions and it becomes confusing if new versions come too soon. To give everyone enough time to review your changes and to not have unnecessary patch versions, its better to wait for a few days, collect the feedback and address it together. We also need to mention what we changed since last time, to help reviewers know if their comments got addressed.
>
> For single patch, we usually don't add a cover letter. You can refer
> https://lore.kernel.org/all/20250102145243.2088-1-namjain@linux.microsoft.com/
> The comments between the two "---" does not make it to git log.
>
>>
>>
>>> 4. In the commit msg of v3, it is still not very clear what problem, you
>>> are trying to fix here. Do you mean to say that fcopy does not work on
>>> Linux? Or you are assuming it won't work and fixing some generic
>>> problem?
>>> Fcopy is supposed to work fine on Linux VM on HyperV with windows host. If there are some errors, please share in cover letter/comments
>>> in the patch along with steps of execution.
>>
>> I have a problem on my PC:
>>
>> Host: Windows 11 Pro (24H2 build 26100.4202)
>> Guest: Fedora 42
>> - kernel-6.14.4-300.fc42.x86_64
>> - hypervfcopyd-6.10-1.fc42.x86_64
>>
>> How to reproduce: run Copy-VMFile commandlet on Host:
>>
>> Following log is in Japanese because my PC is set to Japanese, sorry.
>> But it says fcopy could not transfer file (test.ps1) to /tmp/ on Linux guest because it already exists.
>> I confirmed /tmp/test.ps1 does not exist of course.
>>
>> ```
>>> Copy-VMFile
>>
>> cmdlet Copy-VMFile at command pipeline position 1
>> Supply values for the following parameters:
>> Name[0]: Fedora42
>> Name[1]:
>> SourcePath: test.ps1
>> DestinationPath: /tmp/
>> FileSource: Host
>> Copy-VMFile: ゲストへのファイルのコピーを開始できませんでした。
>>
>> ソース ファイル 'C:\test\test.ps1' をゲストの宛先 '/tmp/' にコピーできま せんでした。
>>
>> 'Fedora42' はゲスト: ファイルがあります。 (0x80070050) へのファイルのコ ピーを開始できませんでした。(仮想マシン ID 9BFDF23D-CCAA-4748- A770-6D654E09A133)
>>
>> 'Fedora42' は、コピー元ファイル 'C:\test\test.ps1' をゲスト上のコピー先 '/tmp/' にコピーできませんでした: ファイルがあります。 (0x80070050)。(仮 想マシン ID 9BFDF23D-CCAA-4748-A770-6D654E09A133)
>> ```
>>
>> I got following fcopy log from journald - it is strange because "/tmp/ test.ps1" should be shown here.
>> ```
>> 6月 04 17:48:24 fc42 HV_UIO_FCOPY[1080]: File: / exists
>> ```
>>
>> As I wrote in commit message, wchar_t is 32 bit in Linux. I confirmed it with "sizeof(wchar_t)".
>> However fcopyd handles it as 16 bit value (__u16), thus I think this is a bug in fcopy, and
>> I think it would also not work on other environments.
>>
>> Actually it works fine with my patch to handle values as 16 bit.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2025/06/04 15:19, Naman Jain wrote:
>>>
>>>
>>> On 6/4/2025 5:13 AM, yasuenag@gmail.com wrote:
>>>> 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 */
>
> Please add a syslog for this. It will help in debugging issues.
>
>>>> + 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;
>
> While this also works, I think you can add it to the while loop itself
> while (len < dest_size && src[len])
>
> Is this related to the FCopy issue that you see or this is just a fix for correct destination file name?
>
>
>>>> 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;
>>>> -
>
> Please help me understand if this is correct. The actual problem you are highlighting lies here when sizeof(wchat_t) is more than 16, i.e. 32?
>
>>>> - 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);
>>>> }
>>>
>>> Hi,
>>> I understand this is your first patch for upstreaming. Here are a few
>>> things you should consider:
>>> 1. Create a new patch file for every new version and then send it.
>>> Currently it seems you are manually editing the same patch file in the
>>> subject and sending it, so each patch version is showing up in the same
>>> thread.
>>> 2. Read, re-read, absorb the information in the link that Easwar also
>>> mentioned:
>>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>>> 3. Keep a minimum of 1-2 weeks gap between successive patch versions to
>>> give time to people to review your changes.
>>> 4. In the commit msg of v3, it is still not very clear what problem, you
>>> are trying to fix here. Do you mean to say that fcopy does not work on
>>> Linux? Or you are assuming it won't work and fixing some generic
>>> problem?
>>> Fcopy is supposed to work fine on Linux VM on HyperV with windows host. If there are some errors, please share in cover letter/comments
>>> in the patch along with steps of execution.
>>>
>>> 5. If its a fix, we should have a proper Fixes tag with the commit you
>>> are fixing.
>>> 6. Have a look at existing conversations at lore to get to know common
>>> practices with single patch, multi patch, cover letters etc.
>>> https://lore.kernel.org/linux-hyperv/?q=linux-hyperv
>>>
>>> Regards,
>>> Naman
>>>
>
>
> Regards,
> Naman
>>>
>
^ permalink raw reply
* Re: [PATCH hyperv-next v3 01/15] Documentation: hyperv: Confidential VMBus
From: Jonathan Corbet @ 2025-06-04 12:53 UTC (permalink / raw)
To: Roman Kisel, alok.a.tiwari, arnd, bp, dave.hansen, decui,
haiyangz, hpa, kys, mingo, mhklinux, tglx, wei.liu, linux-arch,
linux-doc, linux-hyperv, linux-kernel, x86
Cc: apais, benhill, bperkins, sunilmut
In-Reply-To: <20250604004341.7194-2-romank@linux.microsoft.com>
Roman Kisel <romank@linux.microsoft.com> writes:
> Define what the confidential VMBus is and describe what advantages
> it offers on the capable hardware.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> ---
> Documentation/virt/hyperv/coco.rst | 125 ++++++++++++++++++++++++++++-
> 1 file changed, 124 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/virt/hyperv/coco.rst b/Documentation/virt/hyperv/coco.rst
> index c15d6fe34b4e..b4904b64219d 100644
> --- a/Documentation/virt/hyperv/coco.rst
> +++ b/Documentation/virt/hyperv/coco.rst
[...]
> +
> +Here is the data flow for a conventional VMBus connection and the Confidential
> +VMBus connection (C stands for the client or VSC, S for the server or VSP):
> +
> ++---- GUEST ----+ +----- DEVICE ----+ +----- HOST -----+
> +| | | | | |
> +| | | | | |
> +| | | ========== |
> +| | | | | |
> +| | | | | |
> +| | | | | |
> ++----- C -------+ +-----------------+ +------- S ------+
This will generate all kinds of errors during the docs build and will
not give the results you are looking for - please actually build the
docs after making a change like this.
You need to put the diagram into a literal block.
Thanks,
jon
^ permalink raw reply
* Re: [PATCH v3 1/1] hv_fcopy_uio_daemon: Fix file copy failure between Windows host and Linux guest
From: Naman Jain @ 2025-06-04 12:36 UTC (permalink / raw)
To: Yasumasa Suenaga, eahariha
Cc: kys, haiyangz, wei.liu, decui, linux-hyperv, ssengar
In-Reply-To: <72f78577-ec5e-43a3-9378-a77e003b05a6@gmail.com>
On 6/4/2025 2:40 PM, Yasumasa Suenaga wrote:
> Hi Naman,
> Sorry for bothering you, but I have some questions:
>
>
Hi,
Happy to help. I have tested your patch on my setup, and it did not
break anything for me. Please find response and additional questions inline.
>> 1. Create a new patch file for every new version and then send it.
>> Currently it seems you are manually editing the same patch file in the
>> subject and sending it, so each patch version is showing up in the same
>> thread.
>
> I've committed changes with "git amend" and created formatted patch with
> "git format-patch",
> then I sent all of *.patch files via "git send-email".
> Do you mean I should commit to fix commits reviewers and send cover
> letter and incremental
> patches only? I couldn't find about it from the guide.
> I checked linux-hyperv list, all of patches have been sent in each
> version AFAICS.
>
* make changes
* git add, git commit, git format-patch, git send-email <path to v1 patch)
* make changes for v2
* git add, git commit --amend, git format-patch -v2, git send-email
<path to v2 patch *only*>
and so on.
I follow above practice, and the patches go in new threads. I am not
sure why they are landing in same thread for you. Anyhow, its not a big
thing.
>
>> 3. Keep a minimum of 1-2 weeks gap between successive patch versions to
>> give time to people to review your changes.
>
> Does it include changes of commit message too?
Other reviewers may be reviewing older versions and it becomes confusing
if new versions come too soon. To give everyone enough time to review
your changes and to not have unnecessary patch versions, its better to
wait for a few days, collect the feedback and address it together. We
also need to mention what we changed since last time, to help reviewers
know if their comments got addressed.
For single patch, we usually don't add a cover letter. You can refer
https://lore.kernel.org/all/20250102145243.2088-1-namjain@linux.microsoft.com/
The comments between the two "---" does not make it to git log.
>
>
>> 4. In the commit msg of v3, it is still not very clear what problem, you
>> are trying to fix here. Do you mean to say that fcopy does not work on
>> Linux? Or you are assuming it won't work and fixing some generic
>> problem?
>> Fcopy is supposed to work fine on Linux VM on HyperV with windows
>> host. If there are some errors, please share in cover letter/comments
>> in the patch along with steps of execution.
>
> I have a problem on my PC:
>
> Host: Windows 11 Pro (24H2 build 26100.4202)
> Guest: Fedora 42
> - kernel-6.14.4-300.fc42.x86_64
> - hypervfcopyd-6.10-1.fc42.x86_64
>
> How to reproduce: run Copy-VMFile commandlet on Host:
>
> Following log is in Japanese because my PC is set to Japanese, sorry.
> But it says fcopy could not transfer file (test.ps1) to /tmp/ on Linux
> guest because it already exists.
> I confirmed /tmp/test.ps1 does not exist of course.
>
> ```
>> Copy-VMFile
>
> cmdlet Copy-VMFile at command pipeline position 1
> Supply values for the following parameters:
> Name[0]: Fedora42
> Name[1]:
> SourcePath: test.ps1
> DestinationPath: /tmp/
> FileSource: Host
> Copy-VMFile: ゲストへのファイルのコピーを開始できませんでした。
>
> ソース ファイル 'C:\test\test.ps1' をゲストの宛先 '/tmp/' にコピーできま
> せんでした。
>
> 'Fedora42' はゲスト: ファイルがあります。 (0x80070050) へのファイルのコ
> ピーを開始できませんでした。(仮想マシン ID 9BFDF23D-CCAA-4748-
> A770-6D654E09A133)
>
> 'Fedora42' は、コピー元ファイル 'C:\test\test.ps1' をゲスト上のコピー先
> '/tmp/' にコピーできませんでした: ファイルがあります。 (0x80070050)。(仮
> 想マシン ID 9BFDF23D-CCAA-4748-A770-6D654E09A133)
> ```
>
> I got following fcopy log from journald - it is strange because "/tmp/
> test.ps1" should be shown here.
> ```
> 6月 04 17:48:24 fc42 HV_UIO_FCOPY[1080]: File: / exists
> ```
>
> As I wrote in commit message, wchar_t is 32 bit in Linux. I confirmed it
> with "sizeof(wchar_t)".
> However fcopyd handles it as 16 bit value (__u16), thus I think this is
> a bug in fcopy, and
> I think it would also not work on other environments.
>
> Actually it works fine with my patch to handle values as 16 bit.
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2025/06/04 15:19, Naman Jain wrote:
>>
>>
>> On 6/4/2025 5:13 AM, yasuenag@gmail.com wrote:
>>> 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 */
Please add a syslog for this. It will help in debugging issues.
>>> + 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;
While this also works, I think you can add it to the while loop itself
while (len < dest_size && src[len])
Is this related to the FCopy issue that you see or this is just a fix
for correct destination file name?
>>> 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;
>>> -
Please help me understand if this is correct. The actual problem you are
highlighting lies here when sizeof(wchat_t) is more than 16, i.e. 32?
>>> - 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);
>>> }
>>
>> Hi,
>> I understand this is your first patch for upstreaming. Here are a few
>> things you should consider:
>> 1. Create a new patch file for every new version and then send it.
>> Currently it seems you are manually editing the same patch file in the
>> subject and sending it, so each patch version is showing up in the same
>> thread.
>> 2. Read, re-read, absorb the information in the link that Easwar also
>> mentioned:
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>> 3. Keep a minimum of 1-2 weeks gap between successive patch versions to
>> give time to people to review your changes.
>> 4. In the commit msg of v3, it is still not very clear what problem, you
>> are trying to fix here. Do you mean to say that fcopy does not work on
>> Linux? Or you are assuming it won't work and fixing some generic
>> problem?
>> Fcopy is supposed to work fine on Linux VM on HyperV with windows
>> host. If there are some errors, please share in cover letter/comments
>> in the patch along with steps of execution.
>>
>> 5. If its a fix, we should have a proper Fixes tag with the commit you
>> are fixing.
>> 6. Have a look at existing conversations at lore to get to know common
>> practices with single patch, multi patch, cover letters etc.
>> https://lore.kernel.org/linux-hyperv/?q=linux-hyperv
>>
>> Regards,
>> Naman
>>
Regards,
Naman
>>
^ permalink raw reply
* Re: [PATCH v4 03/10] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
From: Ricardo Neri @ 2025-06-04 11:48 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: x86, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley, Saurabh Sengar, Chris Oo, Kirill A. Shutemov,
linux-hyperv, devicetree, linux-acpi, linux-kernel,
Ravi V. Shankar, Ricardo Neri, Yunhong Jiang
In-Reply-To: <CAJZ5v0i6Ej6Tg-4aS_B3Gg2Z5Bk0g_AA9wdG0FQmuq0ZqdP1og@mail.gmail.com>
On Wed, Jun 04, 2025 at 11:18:15AM +0200, Rafael J. Wysocki wrote:
> On Wed, Jun 4, 2025 at 2:18 AM Ricardo Neri
> <ricardo.neri-calderon@linux.intel.com> wrote:
> >
> > Add DeviceTree bindings to enumerate the wakeup mailbox used in platform
> > firmware for Intel processors.
> >
> > x86 platforms commonly boot secondary CPUs using an INIT assert, de-assert
> > followed by Start-Up IPI messages. The wakeup mailbox can be used when this
> > mechanism is unavailable.
> >
> > The wakeup mailbox offers more control to the operating system to boot
> > secondary CPUs than a spin-table. It allows the reuse of same wakeup vector
> > for all CPUs while maintaining control over which CPUs to boot and when.
> > While it is possible to achieve the same level of control using a spin-
> > table, it would require to specify a separate `cpu-release-addr` for each
> > secondary CPU.
> >
> > The operation and structure of the mailbox is described in the
> > Multiprocessor Wakeup Structure defined in the ACPI specification. Note
> > that this structure does not specify how to publish the mailbox to the
> > operating system (ACPI-based platform firmware uses a separate table). No
> > ACPI table is needed in DeviceTree-based firmware to enumerate the mailbox.
> >
> > Add a `compatible` property that the operating system can use to discover
> > the mailbox. Nodes wanting to refer to the reserved memory usually define a
> > `memory-region` property. /cpus/cpu* nodes would want to refer to the
> > mailbox, but they do not have such property defined in the DeviceTree
> > specification. Moreover, it would imply that there is a memory region per
> > CPU.
> >
> > Co-developed-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> > ---
> > Changes since v3:
> > - Removed redefinitions of the mailbox and instead referred to ACPI
> > specification as per discussion on LKML.
> > - Clarified that DeviceTree-based firmware do not require the use of
> > ACPI tables to enumerate the mailbox. (Rob)
> > - Described the need of using a `compatible` property.
> > - Dropped the `alignment` property. (Krzysztof, Rafael)
> > - Used a real address for the mailbox node. (Krzysztof)
> >
> > Changes since v2:
> > - Implemented the mailbox as a reserved-memory node. Add to it a
> > `compatible` property. (Krzysztof)
> > - Explained the relationship between the mailbox and the `enable-mehod`
> > property of the CPU nodes.
> > - Expanded the documentation of the binding.
> >
> > Changes since v1:
> > - Added more details to the description of the binding.
> > - Added requirement a new requirement for cpu@N nodes to add an
> > `enable-method`.
> > ---
> > .../reserved-memory/intel,wakeup-mailbox.yaml | 48 ++++++++++++++++++++++
> > 1 file changed, 48 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> > new file mode 100644
> > index 000000000000..f18643805866
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> > @@ -0,0 +1,48 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/reserved-memory/intel,wakeup-mailbox.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Wakeup Mailbox for Intel processors
> > +
> > +description: |
> > + The Wakeup Mailbox provides a mechanism for the operating system to wake up
> > + secondary CPUs on Intel processors. It is an alternative to the INIT-!INIT-
> > + SIPI sequence used on most x86 systems.
> > +
> > + The structure and operation of the mailbox is described in the Multiprocessor
> > + Wakeup Structure of the ACPI specification.
>
> Please make this more specific: Which specification version and what section.
>
> You may as well add a URL here too.
Sure Rafael. I will refer to the ACPI specification v6.6 secton 5.2.12.19.
It is the latest version at the time of writing this schema.
^ permalink raw reply
* Re: [PATCH v4 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
From: Ricardo Neri @ 2025-06-04 11:44 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: x86, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley, Saurabh Sengar, Chris Oo, Kirill A. Shutemov,
linux-hyperv, devicetree, linux-acpi, linux-kernel,
Ravi V. Shankar, Ricardo Neri, Yunhong Jiang
In-Reply-To: <CAJZ5v0geZAnLRkeunW06JKE1gyDcd15EGzqJ_A-cZHO_koJVAw@mail.gmail.com>
On Wed, Jun 04, 2025 at 11:12:53AM +0200, Rafael J. Wysocki wrote:
> On Wed, Jun 4, 2025 at 2:18 AM Ricardo Neri
> <ricardo.neri-calderon@linux.intel.com> wrote:
> >
> > The bootstrap processor uses acpi_wakeup_cpu() to indicate to firmware that
> > it wants to boot a secondary CPU using a mailbox as described in the
> > Multiprocessor Wakeup Structure of the ACPI specification.
> >
> > The platform firmware may implement the mailbox as described in the ACPI
> > specification but enumerate it using a DeviceTree graph. An example of
> > this is OpenHCL paravisor.
> >
> > Move the code used to setup and use the mailbox for CPU wakeup out of the
> > ACPI directory into a new smpwakeup.c file that both ACPI and DeviceTree
> > can use.
> >
> > No functional changes are intended.
> >
> > Co-developed-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> > ---
> > Changes since v3:
> > - Create a new file smpwakeup.c instead of relocating it to smpboot.c.
> > (Rafael)
> >
> > Changes since v2:
> > - Only move to smpboot.c the portions of the code that configure and
> > use the mailbox. This also resolved the compile warnings about unused
> > functions that Michael Kelley reported.
> > - Edited the commit message for clarity.
> >
> > Changes since v1:
> > - None.
> > ---
> > arch/x86/Kconfig | 7 ++++
> > arch/x86/kernel/Makefile | 1 +
> > arch/x86/kernel/acpi/madt_wakeup.c | 76 ----------------------------------
> > arch/x86/kernel/smpwakeup.c | 83 ++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 91 insertions(+), 76 deletions(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index cb0f4af31789..82147edb355a 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -1113,6 +1113,13 @@ config X86_LOCAL_APIC
> > depends on X86_64 || SMP || X86_UP_APIC || PCI_MSI
> > select IRQ_DOMAIN_HIERARCHY
> >
> > +config X86_MAILBOX_WAKEUP
> > + def_bool y
> > + depends on OF || ACPI_MADT_WAKEUP
>
> At this point the dependency on OF is premature. IMV it should be
> added in a later patch.
I see your point. Sure, I will the dependency in a later patch.
^ permalink raw reply
* Re: [PATCH v4 03/10] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
From: Rafael J. Wysocki @ 2025-06-04 9:18 UTC (permalink / raw)
To: Ricardo Neri
Cc: x86, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley, Rafael J. Wysocki, Saurabh Sengar, Chris Oo,
Kirill A. Shutemov, linux-hyperv, devicetree, linux-acpi,
linux-kernel, Ravi V. Shankar, Ricardo Neri, Yunhong Jiang
In-Reply-To: <20250603-rneri-wakeup-mailbox-v4-3-d533272b7232@linux.intel.com>
On Wed, Jun 4, 2025 at 2:18 AM Ricardo Neri
<ricardo.neri-calderon@linux.intel.com> wrote:
>
> Add DeviceTree bindings to enumerate the wakeup mailbox used in platform
> firmware for Intel processors.
>
> x86 platforms commonly boot secondary CPUs using an INIT assert, de-assert
> followed by Start-Up IPI messages. The wakeup mailbox can be used when this
> mechanism is unavailable.
>
> The wakeup mailbox offers more control to the operating system to boot
> secondary CPUs than a spin-table. It allows the reuse of same wakeup vector
> for all CPUs while maintaining control over which CPUs to boot and when.
> While it is possible to achieve the same level of control using a spin-
> table, it would require to specify a separate `cpu-release-addr` for each
> secondary CPU.
>
> The operation and structure of the mailbox is described in the
> Multiprocessor Wakeup Structure defined in the ACPI specification. Note
> that this structure does not specify how to publish the mailbox to the
> operating system (ACPI-based platform firmware uses a separate table). No
> ACPI table is needed in DeviceTree-based firmware to enumerate the mailbox.
>
> Add a `compatible` property that the operating system can use to discover
> the mailbox. Nodes wanting to refer to the reserved memory usually define a
> `memory-region` property. /cpus/cpu* nodes would want to refer to the
> mailbox, but they do not have such property defined in the DeviceTree
> specification. Moreover, it would imply that there is a memory region per
> CPU.
>
> Co-developed-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Changes since v3:
> - Removed redefinitions of the mailbox and instead referred to ACPI
> specification as per discussion on LKML.
> - Clarified that DeviceTree-based firmware do not require the use of
> ACPI tables to enumerate the mailbox. (Rob)
> - Described the need of using a `compatible` property.
> - Dropped the `alignment` property. (Krzysztof, Rafael)
> - Used a real address for the mailbox node. (Krzysztof)
>
> Changes since v2:
> - Implemented the mailbox as a reserved-memory node. Add to it a
> `compatible` property. (Krzysztof)
> - Explained the relationship between the mailbox and the `enable-mehod`
> property of the CPU nodes.
> - Expanded the documentation of the binding.
>
> Changes since v1:
> - Added more details to the description of the binding.
> - Added requirement a new requirement for cpu@N nodes to add an
> `enable-method`.
> ---
> .../reserved-memory/intel,wakeup-mailbox.yaml | 48 ++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> new file mode 100644
> index 000000000000..f18643805866
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/intel,wakeup-mailbox.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Wakeup Mailbox for Intel processors
> +
> +description: |
> + The Wakeup Mailbox provides a mechanism for the operating system to wake up
> + secondary CPUs on Intel processors. It is an alternative to the INIT-!INIT-
> + SIPI sequence used on most x86 systems.
> +
> + The structure and operation of the mailbox is described in the Multiprocessor
> + Wakeup Structure of the ACPI specification.
Please make this more specific: Which specification version and what section.
You may as well add a URL here too.
> +
> + The implementation of the mailbox in platform firmware is described in the
> + Intel TDX Virtual Firmware Design Guide section 4.3.5.
> +
> + See https://www.intel.com/content/www/us/en/content-details/733585/intel-tdx-virtual-firmware-design-guide.html
> +
> +maintainers:
> + - Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> +
> +allOf:
> + - $ref: reserved-memory.yaml
> +
> +properties:
> + compatible:
> + const: intel,wakeup-mailbox
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <1>;
> +
> + wakeup-mailbox@ffff0000 {
> + compatible = "intel,wakeup-mailbox";
> + reg = <0x0 0xffff0000 0x1000>;
> + };
> + };
>
> --
^ permalink raw reply
* Re: [PATCH v4 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
From: Rafael J. Wysocki @ 2025-06-04 9:12 UTC (permalink / raw)
To: Ricardo Neri
Cc: x86, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley, Rafael J. Wysocki, Saurabh Sengar, Chris Oo,
Kirill A. Shutemov, linux-hyperv, devicetree, linux-acpi,
linux-kernel, Ravi V. Shankar, Ricardo Neri, Yunhong Jiang
In-Reply-To: <20250603-rneri-wakeup-mailbox-v4-2-d533272b7232@linux.intel.com>
On Wed, Jun 4, 2025 at 2:18 AM Ricardo Neri
<ricardo.neri-calderon@linux.intel.com> wrote:
>
> The bootstrap processor uses acpi_wakeup_cpu() to indicate to firmware that
> it wants to boot a secondary CPU using a mailbox as described in the
> Multiprocessor Wakeup Structure of the ACPI specification.
>
> The platform firmware may implement the mailbox as described in the ACPI
> specification but enumerate it using a DeviceTree graph. An example of
> this is OpenHCL paravisor.
>
> Move the code used to setup and use the mailbox for CPU wakeup out of the
> ACPI directory into a new smpwakeup.c file that both ACPI and DeviceTree
> can use.
>
> No functional changes are intended.
>
> Co-developed-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Changes since v3:
> - Create a new file smpwakeup.c instead of relocating it to smpboot.c.
> (Rafael)
>
> Changes since v2:
> - Only move to smpboot.c the portions of the code that configure and
> use the mailbox. This also resolved the compile warnings about unused
> functions that Michael Kelley reported.
> - Edited the commit message for clarity.
>
> Changes since v1:
> - None.
> ---
> arch/x86/Kconfig | 7 ++++
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/acpi/madt_wakeup.c | 76 ----------------------------------
> arch/x86/kernel/smpwakeup.c | 83 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 91 insertions(+), 76 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index cb0f4af31789..82147edb355a 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1113,6 +1113,13 @@ config X86_LOCAL_APIC
> depends on X86_64 || SMP || X86_UP_APIC || PCI_MSI
> select IRQ_DOMAIN_HIERARCHY
>
> +config X86_MAILBOX_WAKEUP
> + def_bool y
> + depends on OF || ACPI_MADT_WAKEUP
At this point the dependency on OF is premature. IMV it should be
added in a later patch.
> + depends on X86_64
> + depends on SMP
> + depends on X86_LOCAL_APIC
> +
> config ACPI_MADT_WAKEUP
> def_bool y
> depends on X86_64
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 99a783fd4691..8f078af42a71 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -94,6 +94,7 @@ apm-y := apm_32.o
> obj-$(CONFIG_APM) += apm.o
> obj-$(CONFIG_SMP) += smp.o
> obj-$(CONFIG_SMP) += smpboot.o
> +obj-$(CONFIG_X86_MAILBOX_WAKEUP) += smpwakeup.o
> obj-$(CONFIG_X86_TSC) += tsc_sync.o
> obj-$(CONFIG_SMP) += setup_percpu.o
> obj-$(CONFIG_X86_MPPARSE) += mpparse.o
> diff --git a/arch/x86/kernel/acpi/madt_wakeup.c b/arch/x86/kernel/acpi/madt_wakeup.c
> index 4033c804307a..a7e0158269b0 100644
> --- a/arch/x86/kernel/acpi/madt_wakeup.c
> +++ b/arch/x86/kernel/acpi/madt_wakeup.c
> @@ -2,12 +2,10 @@
> #include <linux/acpi.h>
> #include <linux/cpu.h>
> #include <linux/delay.h>
> -#include <linux/io.h>
> #include <linux/kexec.h>
> #include <linux/memblock.h>
> #include <linux/pgtable.h>
> #include <linux/sched/hotplug.h>
> -#include <asm/apic.h>
> #include <asm/barrier.h>
> #include <asm/init.h>
> #include <asm/intel_pt.h>
> @@ -15,12 +13,6 @@
> #include <asm/processor.h>
> #include <asm/reboot.h>
>
> -/* Physical address of the Multiprocessor Wakeup Structure mailbox */
> -static u64 acpi_mp_wake_mailbox_paddr __ro_after_init;
> -
> -/* Virtual address of the Multiprocessor Wakeup Structure mailbox */
> -static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox;
> -
> static u64 acpi_mp_pgd __ro_after_init;
> static u64 acpi_mp_reset_vector_paddr __ro_after_init;
>
> @@ -127,63 +119,6 @@ static int __init acpi_mp_setup_reset(u64 reset_vector)
> return 0;
> }
>
> -static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
> -{
> - if (!acpi_mp_wake_mailbox_paddr) {
> - pr_warn_once("No MADT mailbox: cannot bringup secondary CPUs. Booting with kexec?\n");
> - return -EOPNOTSUPP;
> - }
> -
> - /*
> - * Remap mailbox memory only for the first call to acpi_wakeup_cpu().
> - *
> - * Wakeup of secondary CPUs is fully serialized in the core code.
> - * No need to protect acpi_mp_wake_mailbox from concurrent accesses.
> - */
> - if (!acpi_mp_wake_mailbox) {
> - acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr,
> - sizeof(*acpi_mp_wake_mailbox),
> - MEMREMAP_WB);
> - }
> -
> - /*
> - * Mailbox memory is shared between the firmware and OS. Firmware will
> - * listen on mailbox command address, and once it receives the wakeup
> - * command, the CPU associated with the given apicid will be booted.
> - *
> - * The value of 'apic_id' and 'wakeup_vector' must be visible to the
> - * firmware before the wakeup command is visible. smp_store_release()
> - * ensures ordering and visibility.
> - */
> - acpi_mp_wake_mailbox->apic_id = apicid;
> - acpi_mp_wake_mailbox->wakeup_vector = start_ip;
> - smp_store_release(&acpi_mp_wake_mailbox->command,
> - ACPI_MP_WAKE_COMMAND_WAKEUP);
> -
> - /*
> - * Wait for the CPU to wake up.
> - *
> - * The CPU being woken up is essentially in a spin loop waiting to be
> - * woken up. It should not take long for it wake up and acknowledge by
> - * zeroing out ->command.
> - *
> - * ACPI specification doesn't provide any guidance on how long kernel
> - * has to wait for a wake up acknowledgment. It also doesn't provide
> - * a way to cancel a wake up request if it takes too long.
> - *
> - * In TDX environment, the VMM has control over how long it takes to
> - * wake up secondary. It can postpone scheduling secondary vCPU
> - * indefinitely. Giving up on wake up request and reporting error opens
> - * possible attack vector for VMM: it can wake up a secondary CPU when
> - * kernel doesn't expect it. Wait until positive result of the wake up
> - * request.
> - */
> - while (READ_ONCE(acpi_mp_wake_mailbox->command))
> - cpu_relax();
> -
> - return 0;
> -}
> -
> static void acpi_mp_disable_offlining(struct acpi_madt_multiproc_wakeup *mp_wake)
> {
> cpu_hotplug_disable_offlining();
> @@ -246,14 +181,3 @@ int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
>
> 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);
> -}
> -
> -struct acpi_madt_multiproc_wakeup_mailbox *acpi_get_mp_wakeup_mailbox(void)
> -{
> - return acpi_mp_wake_mailbox;
> -}
> diff --git a/arch/x86/kernel/smpwakeup.c b/arch/x86/kernel/smpwakeup.c
> new file mode 100644
> index 000000000000..e34ffbfffaf5
> --- /dev/null
> +++ b/arch/x86/kernel/smpwakeup.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/acpi.h>
> +#include <linux/io.h>
> +#include <linux/printk.h>
> +#include <linux/types.h>
> +#include <asm/apic.h>
> +#include <asm/barrier.h>
> +#include <asm/processor.h>
> +
> +/* Physical address of the Multiprocessor Wakeup Structure mailbox */
> +static u64 acpi_mp_wake_mailbox_paddr __ro_after_init;
> +
> +/* Virtual address of the Multiprocessor Wakeup Structure mailbox */
> +static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox;
> +
> +static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
> +{
> + if (!acpi_mp_wake_mailbox_paddr) {
> + pr_warn_once("No MADT mailbox: cannot bringup secondary CPUs. Booting with kexec?\n");
> + return -EOPNOTSUPP;
> + }
> +
> + /*
> + * Remap mailbox memory only for the first call to acpi_wakeup_cpu().
> + *
> + * Wakeup of secondary CPUs is fully serialized in the core code.
> + * No need to protect acpi_mp_wake_mailbox from concurrent accesses.
> + */
> + if (!acpi_mp_wake_mailbox) {
> + acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr,
> + sizeof(*acpi_mp_wake_mailbox),
> + MEMREMAP_WB);
> + }
> +
> + /*
> + * Mailbox memory is shared between the firmware and OS. Firmware will
> + * listen on mailbox command address, and once it receives the wakeup
> + * command, the CPU associated with the given apicid will be booted.
> + *
> + * The value of 'apic_id' and 'wakeup_vector' must be visible to the
> + * firmware before the wakeup command is visible. smp_store_release()
> + * ensures ordering and visibility.
> + */
> + acpi_mp_wake_mailbox->apic_id = apicid;
> + acpi_mp_wake_mailbox->wakeup_vector = start_ip;
> + smp_store_release(&acpi_mp_wake_mailbox->command,
> + ACPI_MP_WAKE_COMMAND_WAKEUP);
> +
> + /*
> + * Wait for the CPU to wake up.
> + *
> + * The CPU being woken up is essentially in a spin loop waiting to be
> + * woken up. It should not take long for it wake up and acknowledge by
> + * zeroing out ->command.
> + *
> + * ACPI specification doesn't provide any guidance on how long kernel
> + * has to wait for a wake up acknowledgment. It also doesn't provide
> + * a way to cancel a wake up request if it takes too long.
> + *
> + * In TDX environment, the VMM has control over how long it takes to
> + * wake up secondary. It can postpone scheduling secondary vCPU
> + * indefinitely. Giving up on wake up request and reporting error opens
> + * possible attack vector for VMM: it can wake up a secondary CPU when
> + * kernel doesn't expect it. Wait until positive result of the wake up
> + * request.
> + */
> + while (READ_ONCE(acpi_mp_wake_mailbox->command))
> + cpu_relax();
> +
> + 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);
> +}
> +
> +struct acpi_madt_multiproc_wakeup_mailbox *acpi_get_mp_wakeup_mailbox(void)
> +{
> + return acpi_mp_wake_mailbox;
> +}
>
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v3 1/1] hv_fcopy_uio_daemon: Fix file copy failure between Windows host and Linux guest
From: Yasumasa Suenaga @ 2025-06-04 9:10 UTC (permalink / raw)
To: Naman Jain, eahariha; +Cc: kys, haiyangz, wei.liu, decui, linux-hyperv, ssengar
In-Reply-To: <581033c2-4ff4-44c8-a33c-02da3461fb51@linux.microsoft.com>
Hi Naman,
Sorry for bothering you, but I have some questions:
> 1. Create a new patch file for every new version and then send it.
> Currently it seems you are manually editing the same patch file in the
> subject and sending it, so each patch version is showing up in the same
> thread.
I've committed changes with "git amend" and created formatted patch with "git format-patch",
then I sent all of *.patch files via "git send-email".
Do you mean I should commit to fix commits reviewers and send cover letter and incremental
patches only? I couldn't find about it from the guide.
I checked linux-hyperv list, all of patches have been sent in each version AFAICS.
> 3. Keep a minimum of 1-2 weeks gap between successive patch versions to
> give time to people to review your changes.
Does it include changes of commit message too?
> 4. In the commit msg of v3, it is still not very clear what problem, you
> are trying to fix here. Do you mean to say that fcopy does not work on
> Linux? Or you are assuming it won't work and fixing some generic
> problem?
> Fcopy is supposed to work fine on Linux VM on HyperV with windows host. If there are some errors, please share in cover letter/comments
> in the patch along with steps of execution.
I have a problem on my PC:
Host: Windows 11 Pro (24H2 build 26100.4202)
Guest: Fedora 42
- kernel-6.14.4-300.fc42.x86_64
- hypervfcopyd-6.10-1.fc42.x86_64
How to reproduce: run Copy-VMFile commandlet on Host:
Following log is in Japanese because my PC is set to Japanese, sorry.
But it says fcopy could not transfer file (test.ps1) to /tmp/ on Linux guest because it already exists.
I confirmed /tmp/test.ps1 does not exist of course.
```
> Copy-VMFile
cmdlet Copy-VMFile at command pipeline position 1
Supply values for the following parameters:
Name[0]: Fedora42
Name[1]:
SourcePath: test.ps1
DestinationPath: /tmp/
FileSource: Host
Copy-VMFile: ゲストへのファイルのコピーを開始できませんでした。
ソース ファイル 'C:\test\test.ps1' をゲストの宛先 '/tmp/' にコピーできませんでした。
'Fedora42' はゲスト: ファイルがあります。 (0x80070050) へのファイルのコピーを開始できませんでした。(仮想マシン ID 9BFDF23D-CCAA-4748-A770-6D654E09A133)
'Fedora42' は、コピー元ファイル 'C:\test\test.ps1' をゲスト上のコピー先 '/tmp/' にコピーできませんでした: ファイルがあります。 (0x80070050)。(仮想マシン ID 9BFDF23D-CCAA-4748-A770-6D654E09A133)
```
I got following fcopy log from journald - it is strange because "/tmp/test.ps1" should be shown here.
```
6月 04 17:48:24 fc42 HV_UIO_FCOPY[1080]: File: / exists
```
As I wrote in commit message, wchar_t is 32 bit in Linux. I confirmed it with "sizeof(wchar_t)".
However fcopyd handles it as 16 bit value (__u16), thus I think this is a bug in fcopy, and
I think it would also not work on other environments.
Actually it works fine with my patch to handle values as 16 bit.
Thanks,
Yasumasa
On 2025/06/04 15:19, Naman Jain wrote:
>
>
> On 6/4/2025 5:13 AM, yasuenag@gmail.com wrote:
>> 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);
>> }
>
> Hi,
> I understand this is your first patch for upstreaming. Here are a few
> things you should consider:
> 1. Create a new patch file for every new version and then send it.
> Currently it seems you are manually editing the same patch file in the
> subject and sending it, so each patch version is showing up in the same
> thread.
> 2. Read, re-read, absorb the information in the link that Easwar also
> mentioned:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> 3. Keep a minimum of 1-2 weeks gap between successive patch versions to
> give time to people to review your changes.
> 4. In the commit msg of v3, it is still not very clear what problem, you
> are trying to fix here. Do you mean to say that fcopy does not work on
> Linux? Or you are assuming it won't work and fixing some generic
> problem?
> Fcopy is supposed to work fine on Linux VM on HyperV with windows host. If there are some errors, please share in cover letter/comments
> in the patch along with steps of execution.
>
> 5. If its a fix, we should have a proper Fixes tag with the commit you
> are fixing.
> 6. Have a look at existing conversations at lore to get to know common
> practices with single patch, multi patch, cover letters etc.
> https://lore.kernel.org/linux-hyperv/?q=linux-hyperv
>
> Regards,
> Naman
>
>
^ permalink raw reply
* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Thomas Zimmermann @ 2025-06-04 8:12 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: <SN6PR02MB4157871127ED95AD24EDF96DD46DA@SN6PR02MB4157.namprd02.prod.outlook.com>
Hi
Am 03.06.25 um 19:50 schrieb Michael Kelley:
> 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.
We (DRM upstream) recently considered moving some fbdev drivers to
drivers/staging or marking them with !DRM if a DRM driver is available.
Hyverv_fb would be a candidate.
At least at SUSE, we ship hypervdrm instead of hyperv_fb. This works
well on the various generations of the hyperv system. Much of our
userspace would not be able to use hyperv_fb anyway.
>
> 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.
The panic output is a feature that we recently added to the kernel. It
allows a DRM driver to display a final error message in the case of a
kernel panic (think of blue screens on Windows). Drivers require a
minimum of support to make it work. That's what the hypervdrm patches
were about.
Best regards
Thomas
>
> 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
--
--
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
* Re: [PATCH v3 1/1] hv_fcopy_uio_daemon: Fix file copy failure between Windows host and Linux guest
From: Naman Jain @ 2025-06-04 6:19 UTC (permalink / raw)
To: yasuenag, eahariha; +Cc: kys, haiyangz, wei.liu, decui, linux-hyperv, ssengar
In-Reply-To: <20250603234300.1997-2-yasuenag@gmail.com>
On 6/4/2025 5:13 AM, yasuenag@gmail.com wrote:
> 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);
> }
Hi,
I understand this is your first patch for upstreaming. Here are a few
things you should consider:
1. Create a new patch file for every new version and then send it.
Currently it seems you are manually editing the same patch file in the
subject and sending it, so each patch version is showing up in the same
thread.
2. Read, re-read, absorb the information in the link that Easwar also
mentioned:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
3. Keep a minimum of 1-2 weeks gap between successive patch versions to
give time to people to review your changes.
4. In the commit msg of v3, it is still not very clear what problem, you
are trying to fix here. Do you mean to say that fcopy does not work on
Linux? Or you are assuming it won't work and fixing some generic
problem?
Fcopy is supposed to work fine on Linux VM on HyperV with windows host.
If there are some errors, please share in cover letter/comments
in the patch along with steps of execution.
5. If its a fix, we should have a proper Fixes tag with the commit you
are fixing.
6. Have a look at existing conversations at lore to get to know common
practices with single patch, multi patch, cover letters etc.
https://lore.kernel.org/linux-hyperv/?q=linux-hyperv
Regards,
Naman
^ permalink raw reply
* Re: Subject: [PATCH v2] vmbus: retrieve connection-id from DeviceTree
From: Saurabh Singh Sengar @ 2025-06-04 3:27 UTC (permalink / raw)
To: Hardik Garg
Cc: kys, haiyangz, wei.liu, decui, ssengar, romank, linux-hyperv,
linux-kernel, stable, apais
In-Reply-To: <096edaf7-cc90-42b6-aff4-c5f088574e1e@linux.microsoft.com>
On Tue, Jun 03, 2025 at 02:01:00PM -0700, Hardik Garg wrote:
> 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
Property need to be documented in Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
- Saurabh
>
^ permalink raw reply
* Re: [PATCH hyperv-next v3 01/15] Documentation: hyperv: Confidential VMBus
From: Randy Dunlap @ 2025-06-04 2:56 UTC (permalink / raw)
To: Roman Kisel, alok.a.tiwari, arnd, bp, corbet, dave.hansen, decui,
haiyangz, hpa, kys, mingo, mhklinux, tglx, wei.liu, linux-arch,
linux-doc, linux-hyperv, linux-kernel, x86
Cc: apais, benhill, bperkins, sunilmut
In-Reply-To: <20250604004341.7194-2-romank@linux.microsoft.com>
Hi--
On 6/3/25 5:43 PM, Roman Kisel wrote:
> Define what the confidential VMBus is and describe what advantages
> it offers on the capable hardware.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> ---
> Documentation/virt/hyperv/coco.rst | 125 ++++++++++++++++++++++++++++-
> 1 file changed, 124 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/virt/hyperv/coco.rst b/Documentation/virt/hyperv/coco.rst
> index c15d6fe34b4e..b4904b64219d 100644
> --- a/Documentation/virt/hyperv/coco.rst
> +++ b/Documentation/virt/hyperv/coco.rst
> @@ -178,7 +178,7 @@ These Hyper-V and VMBus memory pages are marked as decrypted:
>
> * VMBus monitor pages
>
> -* Synthetic interrupt controller (synic) related pages (unless supplied by
> +* Synthetic interrupt controller (SynIC) related pages (unless supplied by
> the paravisor)
>
> * Per-cpu hypercall input and output pages (unless running with a paravisor)
> @@ -258,3 +258,126 @@ normal page fault is generated instead of #VC or #VE, and the page-fault-
> based handlers for load_unaligned_zeropad() fixup the reference. When the
> encrypted/decrypted transition is complete, the pages are marked as "present"
> again. See hv_vtom_clear_present() and hv_vtom_set_host_visibility().
> +
> +Confidential VMBus
> +------------------
> +
> +The confidential VMBus enables the confidential guest not to interact with the
> +untrusted host partition and the untrusted hypervisor. Instead, the guest relies
> +on the trusted paravisor to communicate with the devices processing sensitive
> +data. The hardware (SNP or TDX) encrypts the guest memory and the register state
> +while measuring the paravisor image using the platform security processor to
> +ensure trusted and confidential computing.
> +
> +Confidential VMBus provides a secure communication channel between the guest and
> +the paravisor, ensuring that sensitive data is protected from hypervisor-level
^^
s/ / /
> +access through memory encryption and register state isolation.
> +
> +The unencrypted data never leaves the VM so neither the host partition nor the
> +hypervisor can access it at all. In addition to that, the guest only needs to
> +establish a VMBus connection with the paravisor for the channels that process
> +sensitive data, and the paravisor abstracts the details of communicating with
> +the specific devices away.
> +
> +Confidential VMBus is an extension of Confidential Computing (CoCo) VMs
> +(a.k.a. "Isolated" VMs in Hyper-V terminology). Without Confidential VMBus,
> +guest VMBus device drivers (the "VSC"s in VMBus terminology) communicate
> +with VMBus servers (the VSPs) running on the Hyper-V host. The
> +communication must be through memory that has been decrypted so the
> +host can access it. With Confidential VMBus, one or more of the VSPs reside
> +in the trusted paravisor layer in the guest VM. Since the paravisor layer also
> +operates in encrypted memory, the memory used for communication with
> +such VSPs does not need to be decrypted and thereby exposed to the
> +Hyper-V host. The paravisor is responsible for communicating securely
> +with the Hyper-V host as necessary. In some cases (e.g. time synchonization,
> +key-value pairs exchange) the unencrypted data doesn't need to be communicated
> +with the host at all, and a conventional VMBus connection suffices.
> +
> +Here is the data flow for a conventional VMBus connection and the Confidential
> +VMBus connection (C stands for the client or VSC, S for the server or VSP):
> +
> ++---- GUEST ----+ +----- DEVICE ----+ +----- HOST -----+
> +| | | | | |
> +| | | | | |
> +| | | ========== |
> +| | | | | |
> +| | | | | |
> +| | | | | |
> ++----- C -------+ +-----------------+ +------- S ------+
> + || ||
> + || ||
> ++------||------------------ VMBus --------------------------||------+
> +| Interrupts, MMIO |
> ++-------------------------------------------------------------------+
> +
> ++---- GUEST --------------- VTL0 ------+ +-- DEVICE --+
> +| | | |
> +| +- PARAVISOR --------- VTL2 -----+ | | |
> +| | +-- VMBus Relay ------+ ====+================ |
> +| | | Interrupts, MMIO | | | | |
> +| | +-------- S ----------+ | | +------------+
> +| | || | |
> +| +---------+ || | |
> +| | Linux | || OpenHCL | |
> +| | kernel | || | |
> +| +---- C --+-----||---------------+ |
> +| || || |
> ++-------++------- C -------------------+ +------------+
> + || | HOST |
> + || +---- S -----+
> ++-------||----------------- VMBus ---------------------------||-----+
> +| Interrupts, MMIO |
> ++-------------------------------------------------------------------+
> +
> +An implementation of the VMBus relay that offers the Confidential VMBus channels
> +is available in the OpenVMM project as a part of the OpenHCL paravisor. Please
> +refer to https://openvmm.dev/ and https://github.com/microsoft/openvmm for more
> +information about the OpenHCL paravisor.
> +
> +A guest that is running with a paravisor must determine at runtime if
> +Confidential VMBus is supported by the current paravisor. It does so by first
> +trying to establish a Confidential VMBus connection with the paravisor using
> +standard mechanisms where the memory remains encrypted. If this succeeds,
> +then the guest can proceed to use Confidential VMBus. If it fails, then the
> +guest must fallback to establishing a non-Confidential VMBus connection with
> +the Hyper-V host.
> +
> +Confidential VMBus is a characteristic of the VMBus connection as a whole,
> +and of each VMBus channel that is created. When a Confidential VMBus
> +connection is established, the paravisor provides the guest the message-passing
> +path that is used for VMBus device creation and deletion, and it provides a
> +per-CPU synthetic interrupt controller (SynIC) just like the SynIC that is
> +offered by the Hyper-V host. Each VMBus device that is offered to the guest
> +indicates the degree to which it participates in Confidential VMBus. The offer
> +indicates if the device uses encrypted ring buffers, and if the device uses
> +encrypted memory for DMA that is done outside the ring buffer. These settings
> +may be different for different devices using the same Confidential VMBus
> +connection.
Various lines throughout:
Please try to keep line lengths to <= 80 characters foe people who read
documentation in a terminal window.
> +Although these settings are separate, in practice it'll always be encrypted
> +ring buffer only or both encrypted ring buffer and external data. If a channel
> +is offered by the paravisor with confidential VMBus, the ring buffer can always
> +be encrypted since it's strictly for communication between the VTL2 paravisor
> +and the VTL0 guest. However, other memory regions are often used for e.g. DMA,
> +so they need to be accessible by the underlying hardware, and must be unencrypted
> +(unless the device supports encrypted memory). Currently, there are no any VSPs
not
> +in OpenHCL that support encrypted external memory, but we will use it in the
> +future.
> +
> +Because some devices on a Confidential VMBus may require decrypted ring buffers
> +and DMA transfers, the guest must interact with two SynICs -- the one provided
> +by the paravisor and the one provided by the Hyper-V host when Confidential
> +VMBus is not offered. Interrupts are always signaled by the paravisor SynIC, but
> +the guest must check for messages and for channel interrupts on both SynICs.
> +
> +In the case of a confidential VM, regular SynIC access by the guest is
> +intercepted by the paravisor (this includes various MSRs such as the SIMP and
> +SIEFP, as well as hypercalls like HvPostMessage and HvSignalEvent). If the guest
> +actually wants to communicate with the hypervisor, it has to use special mechanisms
> +(GHCB page on SNP, or tdcall on TDX). Messages will always be one or the other:
> +with confidential VMBus, all messages use the paravisor SynIC, otherwise they all
> +use the hypervisor SynIC. For interrupt signaling, though, some channels may be
> +running on the host (non-confidential, using the VMBus relay) and use the hypervisor
> +SynIC, and some on the paravisor and use its SynIC. The RelIDs are coordinated by
> +the OpenHCL VMBus server and are guaranteed to be unique regardless of whether
> +the channel originated on the host or the paravisor.
--
~Randy
^ permalink raw reply
* Re: [PATCH v4 03/10] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
From: Rob Herring (Arm) @ 2025-06-04 1:31 UTC (permalink / raw)
To: Ricardo Neri
Cc: Michael Kelley, linux-acpi, Wei Liu, Conor Dooley, Yunhong Jiang,
K. Y. Srinivasan, Chris Oo, Ricardo Neri, linux-hyperv,
Haiyang Zhang, Krzysztof Kozlowski, linux-kernel, Ravi V. Shankar,
Rafael J. Wysocki, Kirill A. Shutemov, x86, Saurabh Sengar,
Dexuan Cui, devicetree
In-Reply-To: <20250603-rneri-wakeup-mailbox-v4-3-d533272b7232@linux.intel.com>
On Tue, 03 Jun 2025 17:15:15 -0700, Ricardo Neri wrote:
> Add DeviceTree bindings to enumerate the wakeup mailbox used in platform
> firmware for Intel processors.
>
> x86 platforms commonly boot secondary CPUs using an INIT assert, de-assert
> followed by Start-Up IPI messages. The wakeup mailbox can be used when this
> mechanism is unavailable.
>
> The wakeup mailbox offers more control to the operating system to boot
> secondary CPUs than a spin-table. It allows the reuse of same wakeup vector
> for all CPUs while maintaining control over which CPUs to boot and when.
> While it is possible to achieve the same level of control using a spin-
> table, it would require to specify a separate `cpu-release-addr` for each
> secondary CPU.
>
> The operation and structure of the mailbox is described in the
> Multiprocessor Wakeup Structure defined in the ACPI specification. Note
> that this structure does not specify how to publish the mailbox to the
> operating system (ACPI-based platform firmware uses a separate table). No
> ACPI table is needed in DeviceTree-based firmware to enumerate the mailbox.
>
> Add a `compatible` property that the operating system can use to discover
> the mailbox. Nodes wanting to refer to the reserved memory usually define a
> `memory-region` property. /cpus/cpu* nodes would want to refer to the
> mailbox, but they do not have such property defined in the DeviceTree
> specification. Moreover, it would imply that there is a memory region per
> CPU.
>
> Co-developed-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Changes since v3:
> - Removed redefinitions of the mailbox and instead referred to ACPI
> specification as per discussion on LKML.
> - Clarified that DeviceTree-based firmware do not require the use of
> ACPI tables to enumerate the mailbox. (Rob)
> - Described the need of using a `compatible` property.
> - Dropped the `alignment` property. (Krzysztof, Rafael)
> - Used a real address for the mailbox node. (Krzysztof)
>
> Changes since v2:
> - Implemented the mailbox as a reserved-memory node. Add to it a
> `compatible` property. (Krzysztof)
> - Explained the relationship between the mailbox and the `enable-mehod`
> property of the CPU nodes.
> - Expanded the documentation of the binding.
>
> Changes since v1:
> - Added more details to the description of the binding.
> - Added requirement a new requirement for cpu@N nodes to add an
> `enable-method`.
> ---
> .../reserved-memory/intel,wakeup-mailbox.yaml | 48 ++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
./Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml:20:111: [warning] line too long (113 > 110 characters) (line-length)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250603-rneri-wakeup-mailbox-v4-3-d533272b7232@linux.intel.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* [PATCH hyperv-next v3 15/15] Drivers: hv: Set the default VMBus version to 6.0
From: Roman Kisel @ 2025-06-04 0:43 UTC (permalink / raw)
To: alok.a.tiwari, arnd, bp, corbet, dave.hansen, decui, haiyangz,
hpa, kys, mingo, mhklinux, tglx, wei.liu, linux-arch, linux-doc,
linux-hyperv, linux-kernel, x86
Cc: apais, benhill, bperkins, sunilmut
In-Reply-To: <20250604004341.7194-1-romank@linux.microsoft.com>
The confidential VMBus is supported by the protocol version
6.0 onwards.
Attempt to establish the VMBus 6.0 connection thus enabling
the confidential VMBus features when available.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
drivers/hv/connection.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index eeb472019d69..7cd43463f969 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -51,6 +51,7 @@ EXPORT_SYMBOL_GPL(vmbus_proto_version);
* Linux guests and are not listed.
*/
static __u32 vmbus_versions[] = {
+ VERSION_WIN10_V6_0,
VERSION_WIN10_V5_3,
VERSION_WIN10_V5_2,
VERSION_WIN10_V5_1,
@@ -65,7 +66,7 @@ static __u32 vmbus_versions[] = {
* Maximal VMBus protocol version guests can negotiate. Useful to cap the
* VMBus version for testing and debugging purpose.
*/
-static uint max_version = VERSION_WIN10_V5_3;
+static uint max_version = VERSION_WIN10_V6_0;
module_param(max_version, uint, S_IRUGO);
MODULE_PARM_DESC(max_version,
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v3 14/15] Drivers: hv: Support establishing the confidential VMBus connection
From: Roman Kisel @ 2025-06-04 0:43 UTC (permalink / raw)
To: alok.a.tiwari, arnd, bp, corbet, dave.hansen, decui, haiyangz,
hpa, kys, mingo, mhklinux, tglx, wei.liu, linux-arch, linux-doc,
linux-hyperv, linux-kernel, x86
Cc: apais, benhill, bperkins, sunilmut
In-Reply-To: <20250604004341.7194-1-romank@linux.microsoft.com>
To establish the confidential VMBus connection the CoCo VM guest
first attempts to connect to the VMBus server run by the paravisor.
If that fails, the guest falls back to the non-confidential VMBus.
Implement that in the VMBus driver initialization.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
drivers/hv/vmbus_drv.c | 169 +++++++++++++++++++++++++++--------------
1 file changed, 110 insertions(+), 59 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index f7e82a4fe133..88701c3ad999 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1057,12 +1057,9 @@ static void vmbus_onmessage_work(struct work_struct *work)
kfree(ctx);
}
-void vmbus_on_msg_dpc(unsigned long data)
+static void __vmbus_on_msg_dpc(void *message_page_addr)
{
- struct hv_per_cpu_context *hv_cpu = (void *)data;
- void *page_addr = hv_cpu->hyp_synic_message_page;
- struct hv_message msg_copy, *msg = (struct hv_message *)page_addr +
- VMBUS_MESSAGE_SINT;
+ struct hv_message msg_copy, *msg;
struct vmbus_channel_message_header *hdr;
enum vmbus_channel_message_type msgtype;
const struct vmbus_channel_message_table_entry *entry;
@@ -1070,6 +1067,10 @@ void vmbus_on_msg_dpc(unsigned long data)
__u8 payload_size;
u32 message_type;
+ if (!message_page_addr)
+ return;
+ msg = (struct hv_message *)message_page_addr + VMBUS_MESSAGE_SINT;
+
/*
* 'enum vmbus_channel_message_type' is supposed to always be 'u32' as
* it is being used in 'struct vmbus_channel_message_header' definition
@@ -1195,6 +1196,14 @@ void vmbus_on_msg_dpc(unsigned long data)
vmbus_signal_eom(msg, message_type);
}
+void vmbus_on_msg_dpc(unsigned long data)
+{
+ struct hv_per_cpu_context *hv_cpu = (void *)data;
+
+ __vmbus_on_msg_dpc(hv_cpu->hyp_synic_message_page);
+ __vmbus_on_msg_dpc(hv_cpu->para_synic_message_page);
+}
+
#ifdef CONFIG_PM_SLEEP
/*
* Fake RESCIND_CHANNEL messages to clean up hv_sock channels by force for
@@ -1233,21 +1242,19 @@ static void vmbus_force_channel_rescinded(struct vmbus_channel *channel)
#endif /* CONFIG_PM_SLEEP */
/*
- * Schedule all channels with events pending
+ * Schedule all channels with events pending.
+ * The event page can be directly checked to get the id of
+ * the channel that has the interrupt pending.
*/
-static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
+static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu, void *event_page_addr)
{
unsigned long *recv_int_page;
u32 maxbits, relid;
+ union hv_synic_event_flags *event;
- /*
- * The event page can be directly checked to get the id of
- * the channel that has the interrupt pending.
- */
- void *page_addr = hv_cpu->hyp_synic_event_page;
- union hv_synic_event_flags *event
- = (union hv_synic_event_flags *)page_addr +
- VMBUS_MESSAGE_SINT;
+ if (!event_page_addr)
+ return;
+ event = (union hv_synic_event_flags *)event_page_addr + VMBUS_MESSAGE_SINT;
maxbits = HV_EVENT_FLAGS_COUNT;
recv_int_page = event->flags;
@@ -1318,26 +1325,40 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
}
}
-static void vmbus_isr(void)
+static void vmbus_message_sched(struct hv_per_cpu_context *hv_cpu, void *message_page_addr)
{
- struct hv_per_cpu_context *hv_cpu
- = this_cpu_ptr(hv_context.cpu_context);
- void *page_addr;
struct hv_message *msg;
- vmbus_chan_sched(hv_cpu);
-
- page_addr = hv_cpu->hyp_synic_message_page;
- msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
+ if (!message_page_addr)
+ return;
+ msg = (struct hv_message *)message_page_addr + VMBUS_MESSAGE_SINT;
/* Check if there are actual msgs to be processed */
if (msg->header.message_type != HVMSG_NONE) {
if (msg->header.message_type == HVMSG_TIMER_EXPIRED) {
hv_stimer0_isr();
vmbus_signal_eom(msg, HVMSG_TIMER_EXPIRED);
- } else
+ } else {
tasklet_schedule(&hv_cpu->msg_dpc);
+ }
}
+}
+
+static void vmbus_isr(void)
+{
+ struct hv_per_cpu_context *hv_cpu
+ = this_cpu_ptr(hv_context.cpu_context);
+
+ /*
+ * Suggested-by: Michael Kelley <mhklinux@outlook.com>
+ * One possible optimization would be to keep track of the largest relID that's in use,
+ * and only scan up to that relID.
+ */
+ vmbus_chan_sched(hv_cpu, hv_cpu->hyp_synic_event_page);
+ vmbus_chan_sched(hv_cpu, hv_cpu->para_synic_event_page);
+
+ vmbus_message_sched(hv_cpu, hv_cpu->hyp_synic_message_page);
+ vmbus_message_sched(hv_cpu, hv_cpu->para_synic_message_page);
add_interrupt_randomness(vmbus_interrupt);
}
@@ -1355,6 +1376,60 @@ static void vmbus_percpu_work(struct work_struct *work)
hv_synic_init(cpu);
}
+static int vmbus_alloc_synic_and_connect(void)
+{
+ int ret, cpu;
+ struct work_struct __percpu *works;
+ int hyperv_cpuhp_online;
+
+ ret = hv_synic_alloc();
+ if (ret < 0)
+ goto err_alloc;
+
+
+ works = alloc_percpu(struct work_struct);
+ if (!works) {
+ ret = -ENOMEM;
+ goto err_alloc;
+ }
+
+ /*
+ * Initialize the per-cpu interrupt state and stimer state.
+ * Then connect to the host.
+ */
+ cpus_read_lock();
+ for_each_online_cpu(cpu) {
+ struct work_struct *work = per_cpu_ptr(works, cpu);
+
+ INIT_WORK(work, vmbus_percpu_work);
+ schedule_work_on(cpu, work);
+ }
+
+ for_each_online_cpu(cpu)
+ flush_work(per_cpu_ptr(works, cpu));
+
+ /* Register the callbacks for possible CPU online/offline'ing */
+ ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online",
+ hv_synic_init, hv_synic_cleanup);
+ cpus_read_unlock();
+ free_percpu(works);
+ if (ret < 0)
+ goto err_alloc;
+ hyperv_cpuhp_online = ret;
+
+ ret = vmbus_connect();
+ if (ret)
+ goto err_connect;
+ return 0;
+
+err_connect:
+ cpuhp_remove_state(hyperv_cpuhp_online);
+ return -ENODEV;
+err_alloc:
+ hv_synic_free();
+ return -ENOMEM;
+}
+
/*
* vmbus_bus_init -Main vmbus driver initialization routine.
*
@@ -1365,8 +1440,7 @@ static void vmbus_percpu_work(struct work_struct *work)
*/
static int vmbus_bus_init(void)
{
- int ret, cpu;
- struct work_struct __percpu *works;
+ int ret;
ret = hv_init();
if (ret != 0) {
@@ -1401,41 +1475,21 @@ static int vmbus_bus_init(void)
}
}
- ret = hv_synic_alloc();
- if (ret)
- goto err_alloc;
-
- works = alloc_percpu(struct work_struct);
- if (!works) {
- ret = -ENOMEM;
- goto err_alloc;
- }
-
/*
- * Initialize the per-cpu interrupt state and stimer state.
- * Then connect to the host.
+ * Attempt to establish the confidential VMBus connection first if this VM is
+ * a hardware confidential VM, and the paravisor is present.
*/
- cpus_read_lock();
- for_each_online_cpu(cpu) {
- struct work_struct *work = per_cpu_ptr(works, cpu);
+ ret = -ENODEV;
+ if (ms_hyperv.paravisor_present && (hv_isolation_type_tdx() || hv_isolation_type_snp())) {
+ is_confidential = true;
+ ret = vmbus_alloc_synic_and_connect();
+ is_confidential = !ret;
- INIT_WORK(work, vmbus_percpu_work);
- schedule_work_on(cpu, work);
+ pr_info("VMBus is confidential: %d\n", is_confidential);
}
- for_each_online_cpu(cpu)
- flush_work(per_cpu_ptr(works, cpu));
-
- /* Register the callbacks for possible CPU online/offline'ing */
- ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online",
- hv_synic_init, hv_synic_cleanup);
- cpus_read_unlock();
- free_percpu(works);
- if (ret < 0)
- goto err_alloc;
- hyperv_cpuhp_online = ret;
-
- ret = vmbus_connect();
+ if (!is_confidential)
+ ret = vmbus_alloc_synic_and_connect();
if (ret)
goto err_connect;
@@ -1451,9 +1505,6 @@ static int vmbus_bus_init(void)
return 0;
err_connect:
- cpuhp_remove_state(hyperv_cpuhp_online);
-err_alloc:
- hv_synic_free();
if (vmbus_irq == -1) {
hv_remove_vmbus_handler();
} else {
--
2.43.0
^ permalink raw reply related
* [PATCH hyperv-next v3 13/15] Drivers: hv: Support confidential VMBus channels
From: Roman Kisel @ 2025-06-04 0:43 UTC (permalink / raw)
To: alok.a.tiwari, arnd, bp, corbet, dave.hansen, decui, haiyangz,
hpa, kys, mingo, mhklinux, tglx, wei.liu, linux-arch, linux-doc,
linux-hyperv, linux-kernel, x86
Cc: apais, benhill, bperkins, sunilmut
In-Reply-To: <20250604004341.7194-1-romank@linux.microsoft.com>
To run a confidential VMBus channels, one has to initialize the
co_ring_buffers and co_external_memory fields of the channel
structure.
Advertise support upon negoatiating the version and compute
values for those fields and initialize them.
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
drivers/hv/channel_mgmt.c | 19 +++++++++++++++++++
drivers/hv/connection.c | 3 +++
2 files changed, 22 insertions(+)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index ca2fe10c110a..33bc29e826bd 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -1021,6 +1021,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
struct vmbus_channel_offer_channel *offer;
struct vmbus_channel *oldchannel, *newchannel;
size_t offer_sz;
+ bool co_ring_buffer, co_external_memory;
offer = (struct vmbus_channel_offer_channel *)hdr;
@@ -1033,6 +1034,22 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
return;
}
+ co_ring_buffer = is_co_ring_buffer(offer);
+ if (co_ring_buffer) {
+ if (vmbus_proto_version < VERSION_WIN10_V6_0 || !vmbus_is_confidential()) {
+ atomic_dec(&vmbus_connection.offer_in_progress);
+ return;
+ }
+ }
+
+ co_external_memory = is_co_external_memory(offer);
+ if (is_co_external_memory(offer)) {
+ if (vmbus_proto_version < VERSION_WIN10_V6_0 || !vmbus_is_confidential()) {
+ atomic_dec(&vmbus_connection.offer_in_progress);
+ return;
+ }
+ }
+
oldchannel = find_primary_channel_by_offer(offer);
if (oldchannel != NULL) {
@@ -1111,6 +1128,8 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
pr_err("Unable to allocate channel object\n");
return;
}
+ newchannel->co_ring_buffer = co_ring_buffer;
+ newchannel->co_external_memory = co_external_memory;
vmbus_setup_channel_state(newchannel, offer);
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index be490c598785..eeb472019d69 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -105,6 +105,9 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID;
}
+ if (vmbus_is_confidential() && version >= VERSION_WIN10_V6_0)
+ msg->feature_flags = VMBUS_FEATURE_FLAG_CONFIDENTIAL_CHANNELS;
+
/*
* shared_gpa_boundary is zero in non-SNP VMs, so it's safe to always
* bitwise OR it
--
2.43.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