Linux-HyperV List
 help / color / mirror / Atom feed
* RE: [EXTERNAL] Re: [PATCH] hv_sock: Return the readable bytes in hvs_stream_has_data()
From: Dexuan Cui @ 2025-07-01 18:37 UTC (permalink / raw)
  To: Paolo Abeni, niuxuewei97@gmail.com, KY Srinivasan, Haiyang Zhang,
	wei.liu@kernel.org, sgarzare@redhat.com, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, horms@kernel.org,
	linux-hyperv@vger.kernel.org, virtualization@lists.linux.dev,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <346e4b8a-2e62-420b-9816-6a35b8b63da1@redhat.com>

> From: Paolo Abeni <pabeni@redhat.com>
> Sent: Tuesday, July 1, 2025 6:05 AM
> ...
> >  static s64 hvs_stream_has_data(struct vsock_sock *vsk)
> >  {
> >  	struct hvsock *hvs = vsk->trans;
> > +	bool need_refill = !hvs->recv_desc;
> >  	s64 ret;
> 
> Minor nit: when reposting please respect the reverse christmas tree
> order above moving 'need_refill' initialization after the following 'if'
> statement.
> 
> /P

Thanks! Will do.

^ permalink raw reply

* Re: [PATCH] hv_sock: Return the readable bytes in hvs_stream_has_data()
From: Paolo Abeni @ 2025-07-01 13:04 UTC (permalink / raw)
  To: Dexuan Cui, niuxuewei97, kys, haiyangz, wei.liu, sgarzare, davem,
	edumazet, kuba, horms, linux-hyperv, virtualization, netdev,
	linux-kernel
In-Reply-To: <1751013889-4951-1-git-send-email-decui@microsoft.com>

On 6/27/25 10:44 AM, Dexuan Cui wrote:
> When hv_sock was originally added, __vsock_stream_recvmsg() and
> vsock_stream_has_data() actually only needed to know whether there
> is any readable data or not, so hvs_stream_has_data() was written to
> return 1 or 0 for simplicity.
> 
> However, now hvs_stream_has_data() should return the readable bytes
> because vsock_data_ready() -> vsock_stream_has_data() needs to know the
> actual bytes rather than a boolean value of 1 or 0.
> 
> The SIOCINQ ioctl support also needs hvs_stream_has_data() to return
> the readable bytes.
> 
> Let hvs_stream_has_data() return the readable bytes of the payload in
> the next host-to-guest VMBus hv_sock packet.
> 
> Note: there may be multpile incoming hv_sock packets pending in the
> VMBus channel's ringbuffer, but so far there is not a VMBus API that
> allows us to know all the readable bytes in total without reading and
> caching the payload of the multiple packets, so let's just return the
> readable bytes of the next single packet. In the future, we'll either
> add a VMBus API that allows us to know the total readable bytes without
> touching the data in the ringbuffer, or the hv_sock driver needs to
> understand the VMBus packet format and parse the packets directly.
> 
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> 
> Hi maintainers, please don't take the patch for now.
> 
> Hi Xuewei Niu, please help to re-post this patch with the next version
> of your patchset "vsock: Introduce SIOCINQ ioctl support". See
> https://lore.kernel.org/virtualization/BL1PR21MB3115F69C544B0FAA145FA4EABF7BA@BL1PR21MB3115.namprd21.prod.outlook.com/#t
> https://lore.kernel.org/virtualization/20250626050219.1847316-1-niuxuewei.nxw@antgroup.com/
> Feel free to add your Signed-off-by, if you need.
> 
>  net/vmw_vsock/hyperv_transport.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> index 31342ab502b4..64f1290a9ae7 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -694,15 +694,25 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
>  static s64 hvs_stream_has_data(struct vsock_sock *vsk)
>  {
>  	struct hvsock *hvs = vsk->trans;
> +	bool need_refill = !hvs->recv_desc;
>  	s64 ret;

Minor nit: when reposting please respect the reverse christmas tree
order above moving 'need_refill' initialization after the following 'if'
statement.

/P


^ permalink raw reply

* Re: [PATCH v2] tools/hv: fcopy: Fix irregularities with size of ring buffer
From: Olaf Hering @ 2025-07-01 11:15 UTC (permalink / raw)
  To: Naman Jain
  Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
	Michael Kelley, linux-hyperv, Saurabh Sengar
In-Reply-To: <20250701104837.3006-1-namjain@linux.microsoft.com>

[-- Attachment #1: Type: text/plain, Size: 285 bytes --]

Tue,  1 Jul 2025 16:18:37 +0530 Naman Jain <namjain@linux.microsoft.com>:

> +		syslog(LOG_ERR, "Could not determine ring size, using default: %u bytes",
> +		       HV_RING_SIZE_DEFAULT);

I think this is not an actionable error.
Maybe use the default just silently?


Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH v2] tools/hv: fcopy: Fix irregularities with size of ring buffer
From: Naman Jain @ 2025-07-01 10:48 UTC (permalink / raw)
  To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
	Michael Kelley
  Cc: linux-hyperv, linux-kernel, Olaf Hering, Saurabh Sengar,
	Naman Jain

Size of ring buffer, as defined in uio_hv_generic driver, is no longer
fixed to 16 KB. This creates a problem in fcopy, since this size was
hardcoded. With the change in place to make ring sysfs node actually
reflect the size of underlying ring buffer, it is safe to get the size
of ring sysfs file and use it for ring buffer size in fcopy daemon.
Fix the issue of disparity in ring buffer size, by making it dynamic
in fcopy uio daemon.

Cc: stable@vger.kernel.org
Fixes: 0315fef2aff9 ("uio_hv_generic: Align ring size to system page")
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
---
Changes since v1:
https://lore.kernel.org/all/20250620070618.3097-1-namjain@linux.microsoft.com/

* Removed unnecessary type casting in malloc for desc variable (Olaf)
* Added retry mechanisms to avoid potential race conditions (Michael)
* Moved the logic to fetch ring size to a later part in main (Michael)

So Michael, you suggested me to move the above logic after the
/dev/uio<N> entry is successfully opened. But there is some issue
with uio_hv_generic changing interrupt mask, resulting in fcopy daemon
to get stuck in pread forever. Since fcopy is broken as of now, I
thought it is better to take it separately, and let this change go.
Please let me know if you are fine with that.
---
 tools/hv/hv_fcopy_uio_daemon.c | 82 +++++++++++++++++++++++++++++++---
 1 file changed, 75 insertions(+), 7 deletions(-)

diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
index 0198321d14a2..f2e4976ebf28 100644
--- a/tools/hv/hv_fcopy_uio_daemon.c
+++ b/tools/hv/hv_fcopy_uio_daemon.c
@@ -36,6 +36,7 @@
 #define WIN8_SRV_VERSION	(WIN8_SRV_MAJOR << 16 | WIN8_SRV_MINOR)
 
 #define FCOPY_UIO		"/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/uio"
+#define FCOPY_CHANNELS_PATH	"/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/channels"
 
 #define FCOPY_VER_COUNT		1
 static const int fcopy_versions[] = {
@@ -47,9 +48,67 @@ static const int fw_versions[] = {
 	UTIL_FW_VERSION
 };
 
-#define HV_RING_SIZE		0x4000 /* 16KB ring buffer size */
+#define HV_RING_SIZE_DEFAULT	0x4000 /* 16KB ring buffer size default */
 
-static unsigned char desc[HV_RING_SIZE];
+static uint32_t get_ring_buffer_size(void)
+{
+	char ring_path[PATH_MAX];
+	DIR *dir;
+	struct dirent *entry;
+	struct stat st;
+	uint32_t ring_size = 0;
+	int retry_count = 0;
+
+	/* Find the channel directory */
+	dir = opendir(FCOPY_CHANNELS_PATH);
+	if (!dir) {
+		usleep(100 * 1000); /* Avoid race with kernel, wait 100ms and retry once */
+		dir = opendir(FCOPY_CHANNELS_PATH);
+		if (!dir) {
+			syslog(LOG_ERR, "Failed to open channels directory: %s", strerror(errno));
+			return HV_RING_SIZE_DEFAULT;
+		}
+	}
+
+retry_once:
+	while ((entry = readdir(dir)) != NULL) {
+		if (entry->d_type == DT_DIR && strcmp(entry->d_name, ".") != 0 &&
+		    strcmp(entry->d_name, "..") != 0) {
+			snprintf(ring_path, sizeof(ring_path), "%s/%s/ring",
+				 FCOPY_CHANNELS_PATH, entry->d_name);
+
+			if (stat(ring_path, &st) == 0) {
+				/*
+				 * stat returns size of Tx, Rx rings combined,
+				 * so take half of it for individual ring size.
+				 */
+				ring_size = (uint32_t)st.st_size / 2;
+				syslog(LOG_INFO, "Ring buffer size from %s: %u bytes",
+				       ring_path, ring_size);
+				break;
+			}
+		}
+	}
+
+	if (!ring_size && retry_count == 0) {
+		retry_count = 1;
+		rewinddir(dir);
+		usleep(100 * 1000); /* Wait 100ms and retry once */
+		goto retry_once;
+	}
+
+	closedir(dir);
+
+	if (!ring_size) {
+		ring_size = HV_RING_SIZE_DEFAULT;
+		syslog(LOG_ERR, "Could not determine ring size, using default: %u bytes",
+		       HV_RING_SIZE_DEFAULT);
+	}
+
+	return ring_size;
+}
+
+static unsigned char *desc;
 
 static int target_fd;
 static char target_fname[PATH_MAX];
@@ -406,7 +465,7 @@ int main(int argc, char *argv[])
 	int daemonize = 1, long_index = 0, opt, ret = -EINVAL;
 	struct vmbus_br txbr, rxbr;
 	void *ring;
-	uint32_t len = HV_RING_SIZE;
+	uint32_t ring_size, len;
 	char uio_name[NAME_MAX] = {0};
 	char uio_dev_path[PATH_MAX] = {0};
 
@@ -437,6 +496,15 @@ int main(int argc, char *argv[])
 	openlog("HV_UIO_FCOPY", 0, LOG_USER);
 	syslog(LOG_INFO, "starting; pid is:%d", getpid());
 
+	ring_size = get_ring_buffer_size();
+	len = ring_size;
+	desc = malloc(ring_size * sizeof(unsigned char));
+	if (!desc) {
+		syslog(LOG_ERR, "malloc failed for desc buffer");
+		ret = -ENOMEM;
+		goto exit;
+	}
+
 	fcopy_get_first_folder(FCOPY_UIO, uio_name);
 	snprintf(uio_dev_path, sizeof(uio_dev_path), "/dev/%s", uio_name);
 	fcopy_fd = open(uio_dev_path, O_RDWR);
@@ -448,14 +516,14 @@ int main(int argc, char *argv[])
 		goto exit;
 	}
 
-	ring = vmbus_uio_map(&fcopy_fd, HV_RING_SIZE);
+	ring = vmbus_uio_map(&fcopy_fd, ring_size);
 	if (!ring) {
 		ret = errno;
 		syslog(LOG_ERR, "mmap ringbuffer failed; error: %d %s", ret, strerror(ret));
 		goto close;
 	}
-	vmbus_br_setup(&txbr, ring, HV_RING_SIZE);
-	vmbus_br_setup(&rxbr, (char *)ring + HV_RING_SIZE, HV_RING_SIZE);
+	vmbus_br_setup(&txbr, ring, ring_size);
+	vmbus_br_setup(&rxbr, (char *)ring + ring_size, ring_size);
 
 	rxbr.vbr->imask = 0;
 
@@ -472,7 +540,7 @@ int main(int argc, char *argv[])
 			goto close;
 		}
 
-		len = HV_RING_SIZE;
+		len = ring_size;
 		ret = rte_vmbus_chan_recv_raw(&rxbr, desc, &len);
 		if (unlikely(ret <= 0)) {
 			/* This indicates a failure to communicate (or worse) */

base-commit: 1343433ed38923a21425c602e92120a1f1db5f7a
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v5 1/1] tools/hv: fcopy: Fix incorrect file path conversion
From: Naman Jain @ 2025-07-01  9:36 UTC (permalink / raw)
  To: yasuenag; +Cc: eahariha, kys, haiyangz, wei.liu, decui, linux-hyperv, ssengar
In-Reply-To: <20250628022217.1514-2-yasuenag@gmail.com>



On 6/28/2025 7:52 AM, yasuenag@gmail.com wrote:
> From: Yasumasa Suenaga <yasuenag@gmail.com>
> 
> The hv_fcopy_uio_daemon fails to correctly handle file copy requests
> from Windows hosts (e.g. via Copy-VMFile) due to wchar_t size
> differences between Windows and Linux. On Linux, wchar_t is 32 bit,
> whereas Windows uses 16 bit wide characters.
> 
> Fix this by ensuring that file transfers from host to Linux guest
> succeed with correctly decoded file names and paths.
> 
> - Treats file name and path as __u16 arrays, not wchar_t*.
> - Allocates fixed-size buffers (W_MAX_PATH) for converted strings
>    instead of using malloc.
> - Adds a check for target path length to prevent snprintf() buffer
>    overflow.
> 
> Fixes: 82b0945ce2c2 ("tools: hv: Add new fcopy application based on uio driver")
> Signed-off-by: Yasumasa Suenaga <yasuenag@gmail.com>
> ---
>   tools/hv/hv_fcopy_uio_daemon.c | 37 +++++++++++++---------------------
>   1 file changed, 14 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
> index 0198321d1..4b09ed6b6 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)) {
> +		syslog(LOG_ERR, "target file name is too long: %s/%s", path_name, file_name);
> +		goto done;
> +	}
>   
>   	/*
>   	 * Check to see if the path is already in place; if not,
> @@ -270,7 +273,7 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
>   {
>   	size_t len = 0;
>   
> -	while (len < dest_size) {
> +	while (len < dest_size && *src) {
>   		if (src[len] < 0x80)
>   			dest[len++] = (char)(*src++);
>   		else
> @@ -282,27 +285,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);
>   }

LGTM. FYI, Fcopy daemon is broken on some systems currently. Below
change should fix it:

https://lore.kernel.org/all/20250620070618.3097-1-namjain@linux.microsoft.com/


Reviewed-by: Naman Jain <namjain@linux.microsoft.com>

^ permalink raw reply

* Re: [PATCH v5 01/10] x86/acpi: Add a helper functions to setup and access the wakeup mailbox
From: Ricardo Neri @ 2025-06-30 22:51 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, Ricardo Neri
In-Reply-To: <CAJZ5v0i+EOthnNexMs7hm1iX+PY0rCNCHjRgB5r5pJ3tz2aw+w@mail.gmail.com>

On Mon, Jun 30, 2025 at 08:55:25PM +0200, Rafael J. Wysocki wrote:
> s/a helper/helper/ in the subject.

Ugh! I missed this since v3. I corrected it. Thanks for noticing.

> 
> On Sat, Jun 28, 2025 at 5:35 AM Ricardo Neri
> <ricardo.neri-calderon@linux.intel.com> wrote:
> >
> > In preparation to move the functionality to wake secondary CPUs up out of
> > the ACPI code, add two helper functions.
> >
> > The function acpi_setup_mp_wakeup_mailbox() stores the physical address of
> > the mailbox and updates the wakeup_secondary_cpu_64() APIC callback.
> >
> > There is a slight change in behavior: now the APIC callback is updated
> > before configuring CPU hotplug offline behavior. This is fine as the APIC
> > callback continues to be updated unconditionally, regardless of the
> > restriction on CPU offlining.
> >
> > The function acpi_madt_multiproc_wakeup_mailbox() returns a pointer to the
> > mailbox. Use this helper function only in the portions of the code for
> > which the variable acpi_mp_wake_mailbox will be out of scope once it is
> > relocated out of the ACPI directory.
> >
> > The wakeup mailbox is only supported for CONFIG_X86_64 and needed only with
> > CONFIG_SMP=y.
> >
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> 
> With the above nit addressed
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Thank you!

^ permalink raw reply

* Re: [PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
From: Ricardo Neri @ 2025-06-30 22:50 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, Ricardo Neri,
	Yunhong Jiang
In-Reply-To: <CAJZ5v0iw9zi8cZK=2j-fWR4tzYgaSMPYoNHf=F+-51cbkxGYSg@mail.gmail.com>

On Mon, Jun 30, 2025 at 08:58:56PM +0200, Rafael J. Wysocki wrote:
> On Sat, Jun 28, 2025 at 5:35 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.
> 
> IMV "that can be used by both ACPI and DeviceTree" would sound better.

I have taken your rewording suggestion.

> 
> > 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>
> 
> With the above addressed
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Thank you!

^ permalink raw reply

* Re: [PATCH v5 03/10] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
From: Ricardo Neri @ 2025-06-30 22:49 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, Ricardo Neri,
	Yunhong Jiang
In-Reply-To: <CAJZ5v0h_oifyhJq4WiOzS0fcexrjeChJhVyAthfQCX=6v_GumA@mail.gmail.com>

On Mon, Jun 30, 2025 at 09:02:00PM +0200, Rafael J. Wysocki wrote:
> On Sat, Jun 28, 2025 at 5:35 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>
> 
> LGTM from the ACPI specification cross-referencing perspective, so
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Thank you Rafael!

I hope this looks good to Krzysztof and Rob too.

^ permalink raw reply

* RE: [PATCH 1/1] Drivers: hv: Select CONFIG_SYSFB only if EFI is enabled
From: Michael Kelley @ 2025-06-30 20:46 UTC (permalink / raw)
  To: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, deller@gmx.de, javierm@redhat.com,
	arnd@arndb.de
  Cc: linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
	stable@vger.kernel.org
In-Reply-To: <20250613230059.380483-1-mhklinux@outlook.com>

From: mhkelley58@gmail.com <mhkelley58@gmail.com> Sent: Friday, June 13, 2025 4:01 PM
> 
> Commit 96959283a58d ("Drivers: hv: Always select CONFIG_SYSFB
> for Hyper-V guests") selects CONFIG_SYSFB for Hyper-V guests
> so that screen_info is available to the VMBus driver to get
> the location of the framebuffer in Generation 2 VMs. However,
> if CONFIG_HYPERV is enabled but CONFIG_EFI is not, a kernel
> link error results in ARM64 builds because screen_info is
> provided by the EFI firmware interface. While configuring
> an ARM64 Hyper-V guest without EFI isn't useful since EFI is
> required to boot, the configuration is still possible and
> the link error should be prevented.
> 
> Fix this by making the selection of CONFIG_SYSFB conditional
> on CONFIG_EFI being defined. For Generation 1 VMs on x86/x64,
> which don't use EFI, the additional condition is OK because
> such VMs get the framebuffer information via a mechanism
> that doesn't use screen_info.
> 
> Fixes: 96959283a58d ("Drivers: hv: Always select CONFIG_SYSFB for Hyper-V guests")
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Closes: https://lore.kernel.org/linux-hyperv/20250610091810.2638058-1-arnd@kernel.org/

Arnd -- Can you give a "Reviewed-by:" or "Acked-by:" for this fix?  It
needs to get into linux-next and then into Linus' tree before we
get too late in the 6.16 release cycle.

Michael

> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202506080820.1wmkQufc-lkp@intel.com/
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> ---
>  drivers/hv/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> index 1cd188b73b74..57623ca7f350 100644
> --- a/drivers/hv/Kconfig
> +++ b/drivers/hv/Kconfig
> @@ -9,7 +9,7 @@ config HYPERV
>  	select PARAVIRT
>  	select X86_HV_CALLBACK_VECTOR if X86
>  	select OF_EARLY_FLATTREE if OF
> -	select SYSFB if !HYPERV_VTL_MODE
> +	select SYSFB if EFI && !HYPERV_VTL_MODE
>  	help
>  	  Select this option to run Linux as a Hyper-V client operating
>  	  system.
> --
> 2.25.1
> 


^ permalink raw reply

* Re: [PATCH 05/16] PCI: brcmstb: Switch to msi_create_parent_irq_domain()
From: Florian Fainelli @ 2025-06-30 19:18 UTC (permalink / raw)
  To: Nam Cao, Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, linux-pci, linux-kernel, Karthikeyan Mitran,
	Hou Zhiqiang, Thomas Petazzoni, Pali Rohár,
	K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Joyce Ooi,
	Jim Quinlan, Nicolas Saenz Julienne,
	Broadcom internal kernel review list, Ray Jui, Scott Branden,
	Ryder Lee, Jianjun Wang, Marek Vasut, Yoshihiro Shimoda,
	Michal Simek, Daire McNamara, Nirmal Patel, Jonathan Derrick,
	Matthias Brugger, AngeloGioacchino Del Regno, linux-arm-kernel,
	linux-hyperv, linux-rpi-kernel, linux-mediatek, linux-renesas-soc
In-Reply-To: <fa72703e06c2ee2c7554082c7152913eb0dd294f.1750858083.git.namcao@linutronix.de>

On 6/26/25 07:47, Nam Cao wrote:
> Move away from the legacy MSI domain setup, switch to use
> msi_create_parent_irq_domain().
> 
> Signed-off-by: Nam Cao <namcao@linutronix.de>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH 06/16] PCI: iproc: Switch to msi_create_parent_irq_domain()
From: Florian Fainelli @ 2025-06-30 19:17 UTC (permalink / raw)
  To: Nam Cao, Marc Zyngier, Thomas Gleixner, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
	Bjorn Helgaas, linux-pci, linux-kernel, Karthikeyan Mitran,
	Hou Zhiqiang, Thomas Petazzoni, Pali Rohár,
	K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Joyce Ooi,
	Jim Quinlan, Nicolas Saenz Julienne,
	Broadcom internal kernel review list, Ray Jui, Scott Branden,
	Ryder Lee, Jianjun Wang, Marek Vasut, Yoshihiro Shimoda,
	Michal Simek, Daire McNamara, Nirmal Patel, Jonathan Derrick,
	Matthias Brugger, AngeloGioacchino Del Regno, linux-arm-kernel,
	linux-hyperv, linux-rpi-kernel, linux-mediatek, linux-renesas-soc
In-Reply-To: <53946d74caf1fd134a1820eac82c3cf64d48779f.1750858083.git.namcao@linutronix.de>

On 6/26/25 07:47, Nam Cao wrote:
> Move away from the legacy MSI domain setup, switch to use
> msi_create_parent_irq_domain().
> 
> Signed-off-by: Nam Cao <namcao@linutronix.de>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH v5 03/10] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
From: Rafael J. Wysocki @ 2025-06-30 19:02 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, Ricardo Neri, Yunhong Jiang
In-Reply-To: <20250627-rneri-wakeup-mailbox-v5-3-df547b1d196e@linux.intel.com>

On Sat, Jun 28, 2025 at 5:35 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>

LGTM from the ACPI specification cross-referencing perspective, so

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
> Changes since v4:
>  - Specified the version and section of the ACPI spec in which the
>    wakeup mailbox is defined. (Rafael)
>  - Fixed a warning from yamllint about line lengths of URLs.
>
> 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      | 50 ++++++++++++++++++++++
>  1 file changed, 50 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..a80d3bac44c2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> @@ -0,0 +1,50 @@
> +# 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 version 6.6 section 5.2.12.19 [1].
> +
> +  The implementation of the mailbox in platform firmware is described in the
> +  Intel TDX Virtual Firmware Design Guide section 4.3.5 [2].
> +
> +  1: https://uefi.org/specs/ACPI/6.6/05_ACPI_Software_Programming_Model.html#multiprocessor-wakeup-structure
> +  2: 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>;
> +        };
> +    };
>
> --
> 2.43.0
>

^ permalink raw reply

* Re: [PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
From: Rafael J. Wysocki @ 2025-06-30 18:58 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, Ricardo Neri, Yunhong Jiang
In-Reply-To: <20250627-rneri-wakeup-mailbox-v5-2-df547b1d196e@linux.intel.com>

On Sat, Jun 28, 2025 at 5:35 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.

IMV "that can be used by both ACPI and DeviceTree" would sound better.

> 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>

With the above addressed

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
> Changes since v4:
>  - Removed dependency on CONFIG_OF. It will be added in a later patch.
>    (Rafael)
>  - Rebased on v6.16-rc3.
>
> 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 71019b3b54ea..e3009cb59928 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1114,6 +1114,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 ACPI_MADT_WAKEUP
> +       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 0d2a6d953be9..1fce3d20cf2d 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 c3ac5ecf3e7d..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, unsigned int cpu)
> -{
> -       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..5089bcda615d
> --- /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, unsigned int cpu)
> +{
> +       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 v5 01/10] x86/acpi: Add a helper functions to setup and access the wakeup mailbox
From: Rafael J. Wysocki @ 2025-06-30 18:55 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, Ricardo Neri
In-Reply-To: <20250627-rneri-wakeup-mailbox-v5-1-df547b1d196e@linux.intel.com>

s/a helper/helper/ in the subject.

On Sat, Jun 28, 2025 at 5:35 AM Ricardo Neri
<ricardo.neri-calderon@linux.intel.com> wrote:
>
> In preparation to move the functionality to wake secondary CPUs up out of
> the ACPI code, add two helper functions.
>
> The function acpi_setup_mp_wakeup_mailbox() stores the physical address of
> the mailbox and updates the wakeup_secondary_cpu_64() APIC callback.
>
> There is a slight change in behavior: now the APIC callback is updated
> before configuring CPU hotplug offline behavior. This is fine as the APIC
> callback continues to be updated unconditionally, regardless of the
> restriction on CPU offlining.
>
> The function acpi_madt_multiproc_wakeup_mailbox() returns a pointer to the
> mailbox. Use this helper function only in the portions of the code for
> which the variable acpi_mp_wake_mailbox will be out of scope once it is
> relocated out of the ACPI directory.
>
> The wakeup mailbox is only supported for CONFIG_X86_64 and needed only with
> CONFIG_SMP=y.
>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>

With the above nit addressed

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
> Changes since v4:
>  - None
>
> Changes since v3:
>  - Squashed the two first patches of the series into one, both introduce
>    helper functions. (Rafael)
>  - Renamed setup_mp_wakeup_mailbox() as acpi_setup_mp_wakeup_mailbox().
>    (Rafael)
>  - Dropped the function prototype for !CONFIG_X86_64. (Rafael)
>
> Changes since v2:
>  - Introduced this patch.
>
> Changes since v1:
>  - N/A
> ---
>  arch/x86/include/asm/smp.h         |  3 +++
>  arch/x86/kernel/acpi/madt_wakeup.c | 20 +++++++++++++++-----
>  2 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> index 0c1c68039d6f..77dce560a70a 100644
> --- a/arch/x86/include/asm/smp.h
> +++ b/arch/x86/include/asm/smp.h
> @@ -146,6 +146,9 @@ static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
>         return per_cpu(cpu_l2c_shared_map, cpu);
>  }
>
> +void acpi_setup_mp_wakeup_mailbox(u64 addr);
> +struct acpi_madt_multiproc_wakeup_mailbox *acpi_get_mp_wakeup_mailbox(void);
> +
>  #else /* !CONFIG_SMP */
>  #define wbinvd_on_cpu(cpu)     wbinvd()
>  static inline int wbinvd_on_all_cpus(void)
> diff --git a/arch/x86/kernel/acpi/madt_wakeup.c b/arch/x86/kernel/acpi/madt_wakeup.c
> index 6d7603511f52..c3ac5ecf3e7d 100644
> --- a/arch/x86/kernel/acpi/madt_wakeup.c
> +++ b/arch/x86/kernel/acpi/madt_wakeup.c
> @@ -37,6 +37,7 @@ static void acpi_mp_play_dead(void)
>
>  static void acpi_mp_cpu_die(unsigned int cpu)
>  {
> +       struct acpi_madt_multiproc_wakeup_mailbox *mailbox = acpi_get_mp_wakeup_mailbox();
>         u32 apicid = per_cpu(x86_cpu_to_apicid, cpu);
>         unsigned long timeout;
>
> @@ -46,13 +47,13 @@ static void acpi_mp_cpu_die(unsigned int cpu)
>          *
>          * BIOS has to clear 'command' field of the mailbox.
>          */
> -       acpi_mp_wake_mailbox->apic_id = apicid;
> -       smp_store_release(&acpi_mp_wake_mailbox->command,
> +       mailbox->apic_id = apicid;
> +       smp_store_release(&mailbox->command,
>                           ACPI_MP_WAKE_COMMAND_TEST);
>
>         /* Don't wait longer than a second. */
>         timeout = USEC_PER_SEC;
> -       while (READ_ONCE(acpi_mp_wake_mailbox->command) && --timeout)
> +       while (READ_ONCE(mailbox->command) && --timeout)
>                 udelay(1);
>
>         if (!timeout)
> @@ -227,7 +228,7 @@ int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
>
>         acpi_table_print_madt_entry(&header->common);
>
> -       acpi_mp_wake_mailbox_paddr = mp_wake->mailbox_address;
> +       acpi_setup_mp_wakeup_mailbox(mp_wake->mailbox_address);
>
>         if (mp_wake->version >= ACPI_MADT_MP_WAKEUP_VERSION_V1 &&
>             mp_wake->header.length >= ACPI_MADT_MP_WAKEUP_SIZE_V1) {
> @@ -243,7 +244,16 @@ int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
>                 acpi_mp_disable_offlining(mp_wake);
>         }
>
> +       return 0;
> +}
> +
> +void __init acpi_setup_mp_wakeup_mailbox(u64 mailbox_paddr)
> +{
> +       acpi_mp_wake_mailbox_paddr = mailbox_paddr;
>         apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu);
> +}
>
> -       return 0;
> +struct acpi_madt_multiproc_wakeup_mailbox *acpi_get_mp_wakeup_mailbox(void)
> +{
> +       return acpi_mp_wake_mailbox;
>  }
>
> --
> 2.43.0
>

^ permalink raw reply

* Re: [PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
From: Peter Zijlstra @ 2025-06-30 13:26 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Ricardo Neri, 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,
	linux-hyperv, devicetree, linux-acpi, linux-kernel, Ricardo Neri,
	Yunhong Jiang
In-Reply-To: <sh3fz5qlmy2smu74ezibbptxgmlpedzui3c4q6x22jc5w5ik4q@qms3osoxh74t>

On Mon, Jun 30, 2025 at 03:07:08PM +0300, Kirill A. Shutemov wrote:
> On Mon, Jun 30, 2025 at 01:03:16PM +0200, Peter Zijlstra wrote:
> > On Fri, Jun 27, 2025 at 08:35:08PM -0700, Ricardo Neri wrote:
> > 
> > > -	/*
> > > -	 * 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;
> > > -}
> > 
> > > +	while (READ_ONCE(acpi_mp_wake_mailbox->command))
> > > +		cpu_relax();
> > > +
> > > +	return 0;
> > > +}
> > 
> > So I realize this is just code movement at this point, but this will
> > hard lockup the machine if the AP doesn't come up, right?
> 
> Correct.
> 
> > IIRC we have some timeout in the regular SIPI bringup if the AP doesn't
> > respond.
> 
> See the comment.

Doh, reading hard ;-) Thanks!

^ permalink raw reply

* Re: [PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
From: Kirill A. Shutemov @ 2025-06-30 12:07 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ricardo Neri, 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,
	linux-hyperv, devicetree, linux-acpi, linux-kernel, Ricardo Neri,
	Yunhong Jiang
In-Reply-To: <20250630110316.GJ1613376@noisy.programming.kicks-ass.net>

On Mon, Jun 30, 2025 at 01:03:16PM +0200, Peter Zijlstra wrote:
> On Fri, Jun 27, 2025 at 08:35:08PM -0700, Ricardo Neri wrote:
> 
> > -	/*
> > -	 * 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;
> > -}
> 
> > +	while (READ_ONCE(acpi_mp_wake_mailbox->command))
> > +		cpu_relax();
> > +
> > +	return 0;
> > +}
> 
> So I realize this is just code movement at this point, but this will
> hard lockup the machine if the AP doesn't come up, right?

Correct.

> IIRC we have some timeout in the regular SIPI bringup if the AP doesn't
> respond.

See the comment.

In TDX guest case, we need to consider malicious VMM that can postpone
scheduling the target vCPU indefinitely. It can give VMM indirect control
of what the target would run upon wakeup. Like, it can wait until the
guest do kexec and the same start RIP would point non-startup code.

I hope we will get SIPI-based CPU bringup in TDX guest eventually. It will
be more reliable.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

^ permalink raw reply

* Re: [PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
From: Peter Zijlstra @ 2025-06-30 11:03 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, Ricardo Neri, Yunhong Jiang
In-Reply-To: <20250627-rneri-wakeup-mailbox-v5-2-df547b1d196e@linux.intel.com>

On Fri, Jun 27, 2025 at 08:35:08PM -0700, Ricardo Neri wrote:

> -	/*
> -	 * 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;
> -}

> +	while (READ_ONCE(acpi_mp_wake_mailbox->command))
> +		cpu_relax();
> +
> +	return 0;
> +}

So I realize this is just code movement at this point, but this will
hard lockup the machine if the AP doesn't come up, right?

IIRC we have some timeout in the regular SIPI bringup if the AP doesn't
respond.

^ permalink raw reply

* [PATCH v2 2/2] x86/hyperv: Switch to msi_create_parent_irq_domain()
From: Nam Cao @ 2025-06-30 10:26 UTC (permalink / raw)
  To: K . Y . Srinivasan, Marc Zyngier, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H . Peter Anvin, linux-hyperv, linux-kernel
  Cc: Nam Cao
In-Reply-To: <cover.1751277765.git.namcao@linutronix.de>

Move away from the legacy MSI domain setup, switch to use
msi_create_parent_irq_domain().

While doing the conversion, I noticed that hv_irq_compose_msi_msg() is
doing more than it is supposed to (composing message content). The
interrupt allocation bits should be moved into hv_msi_domain_alloc().
However, I have no hardware to test this change, therefore I leave a TODO
note.

Signed-off-by: Nam Cao <namcao@linutronix.de>
---
 arch/x86/hyperv/irqdomain.c | 111 ++++++++++++++++++++++++------------
 drivers/hv/Kconfig          |   1 +
 2 files changed, 77 insertions(+), 35 deletions(-)

diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
index 31f0d29cbc5e3..9b3b65ffbd2e2 100644
--- a/arch/x86/hyperv/irqdomain.c
+++ b/arch/x86/hyperv/irqdomain.c
@@ -10,6 +10,7 @@
 
 #include <linux/pci.h>
 #include <linux/irq.h>
+#include <linux/irqchip/irq-msi-lib.h>
 #include <asm/mshyperv.h>
 
 static int hv_map_interrupt(union hv_device_id device_id, bool level,
@@ -276,59 +277,99 @@ static void hv_teardown_msi_irq(struct pci_dev *dev, struct irq_data *irqd)
 		hv_status_err(status, "\n");
 }
 
-static void hv_msi_free_irq(struct irq_domain *domain,
-			    struct msi_domain_info *info, unsigned int virq)
-{
-	struct irq_data *irqd = irq_get_irq_data(virq);
-	struct msi_desc *desc;
-
-	if (!irqd)
-		return;
-
-	desc = irq_data_get_msi_desc(irqd);
-	if (!desc || !desc->irq || WARN_ON_ONCE(!dev_is_pci(desc->dev)))
-		return;
-
-	hv_teardown_msi_irq(to_pci_dev(desc->dev), irqd);
-}
-
 /*
  * IRQ Chip for MSI PCI/PCI-X/PCI-Express Devices,
  * which implement the MSI or MSI-X Capability Structure.
  */
 static struct irq_chip hv_pci_msi_controller = {
 	.name			= "HV-PCI-MSI",
-	.irq_unmask		= pci_msi_unmask_irq,
-	.irq_mask		= pci_msi_mask_irq,
 	.irq_ack		= irq_chip_ack_parent,
-	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.irq_compose_msi_msg	= hv_irq_compose_msi_msg,
-	.irq_set_affinity	= msi_domain_set_affinity,
-	.flags			= IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MOVE_DEFERRED,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
 };
 
-static struct msi_domain_ops pci_msi_domain_ops = {
-	.msi_free		= hv_msi_free_irq,
-	.msi_prepare		= pci_msi_prepare,
+static bool hv_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
+				 struct irq_domain *real_parent, struct msi_domain_info *info)
+{
+	struct irq_chip *chip = info->chip;
+
+	if (!msi_lib_init_dev_msi_info(dev, domain, real_parent, info))
+		return false;
+
+	chip->flags |= IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MOVE_DEFERRED;
+
+	info->ops->msi_prepare = pci_msi_prepare;
+
+	return true;
+}
+
+#define HV_MSI_FLAGS_SUPPORTED	(MSI_GENERIC_FLAGS_MASK | MSI_FLAG_PCI_MSIX)
+#define HV_MSI_FLAGS_REQUIRED	(MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS)
+
+static struct msi_parent_ops hv_msi_parent_ops = {
+	.supported_flags	= HV_MSI_FLAGS_SUPPORTED,
+	.required_flags		= HV_MSI_FLAGS_REQUIRED,
+	.bus_select_token	= DOMAIN_BUS_NEXUS,
+	.bus_select_mask	= MATCH_PCI_MSI,
+	.chip_flags		= MSI_CHIP_FLAG_SET_ACK,
+	.prefix			= "HV-",
+	.init_dev_msi_info	= hv_init_dev_msi_info,
 };
 
-static struct msi_domain_info hv_pci_msi_domain_info = {
-	.flags		= MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
-			  MSI_FLAG_PCI_MSIX,
-	.ops		= &pci_msi_domain_ops,
-	.chip		= &hv_pci_msi_controller,
-	.handler	= handle_edge_irq,
-	.handler_name	= "edge",
+static int hv_msi_domain_alloc(struct irq_domain *d, unsigned int virq, unsigned int nr_irqs,
+			       void *arg)
+{
+	/*
+	 * TODO: The allocation bits of hv_irq_compose_msi_msg(), i.e. everything except
+	 * entry_to_msi_msg() should be in here.
+	 */
+
+	int ret;
+
+	ret = irq_domain_alloc_irqs_parent(d, virq, nr_irqs, arg);
+	if (ret)
+		return ret;
+
+	for (int i = 0; i < nr_irqs; ++i) {
+		irq_domain_set_info(d, virq + i, 0, &hv_pci_msi_controller, NULL,
+				    handle_edge_irq, NULL, "edge");
+	}
+	return 0;
+}
+
+static void hv_msi_domain_free(struct irq_domain *d, unsigned int virq, unsigned int nr_irqs)
+{
+	for (int i = 0; i < nr_irqs; ++i) {
+		struct irq_data *irqd = irq_domain_get_irq_data(d, virq);
+		struct msi_desc *desc;
+
+		desc = irq_data_get_msi_desc(irqd);
+		if (!desc || !desc->irq || WARN_ON_ONCE(!dev_is_pci(desc->dev)))
+			continue;
+
+		hv_teardown_msi_irq(to_pci_dev(desc->dev), irqd);
+	}
+	irq_domain_free_irqs_top(d, virq, nr_irqs);
+}
+
+static const struct irq_domain_ops hv_msi_domain_ops = {
+	.select	= msi_lib_irq_domain_select,
+	.alloc	= hv_msi_domain_alloc,
+	.free	= hv_msi_domain_free,
 };
 
 struct irq_domain * __init hv_create_pci_msi_domain(void)
 {
 	struct irq_domain *d = NULL;
-	struct fwnode_handle *fn;
 
-	fn = irq_domain_alloc_named_fwnode("HV-PCI-MSI");
-	if (fn)
-		d = pci_msi_create_irq_domain(fn, &hv_pci_msi_domain_info, x86_vector_domain);
+	struct irq_domain_info info = {
+		.fwnode		= irq_domain_alloc_named_fwnode("HV-PCI-MSI"),
+		.ops		= &hv_msi_domain_ops,
+		.parent		= x86_vector_domain,
+	};
+
+	if (info.fwnode)
+		d = msi_create_parent_irq_domain(&info, &hv_msi_parent_ops);
 
 	/* No point in going further if we can't get an irq domain */
 	BUG_ON(!d);
diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index 1cd188b73b743..e62a0f8b34198 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -10,6 +10,7 @@ config HYPERV
 	select X86_HV_CALLBACK_VECTOR if X86
 	select OF_EARLY_FLATTREE if OF
 	select SYSFB if !HYPERV_VTL_MODE
+	select IRQ_MSI_LIB if X86
 	help
 	  Select this option to run Linux as a Hyper-V client operating
 	  system.
-- 
2.39.5


^ permalink raw reply related

* [PATCH v2 1/2] irqchip/irq-msi-lib: Select CONFIG_GENERIC_MSI_IRQ
From: Nam Cao @ 2025-06-30 10:26 UTC (permalink / raw)
  To: K . Y . Srinivasan, Marc Zyngier, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H . Peter Anvin, linux-hyperv, linux-kernel
  Cc: Nam Cao, kernel test robot
In-Reply-To: <cover.1751277765.git.namcao@linutronix.de>

irq-msi-lib directly uses struct msi_domain_info and
MSI_FLAG_ALLOC_SIMPLE_MSI_DESCS (and likely more) which are only available
when CONFIG_GENERIC_MSI_IRQ=y.

However, no dependency is specified. If it happens that
CONFIG_IRQ_MSI_LIB=y while CONFIG_GENERIC_MSI_IRQ=n, the kernel fails to
build.

Specify this dependency and fix the build.

Fixes: 72e257c6f058 ("irqchip: Provide irq-msi-lib")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202506282256.cHlEHrdc-lkp@intel.com/
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
 drivers/irqchip/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 0d196e4471426..c3928ef793449 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -74,6 +74,7 @@ config ARM_VIC_NR
 
 config IRQ_MSI_LIB
 	bool
+	select GENERIC_MSI_IRQ
 
 config ARMADA_370_XP_IRQ
 	bool
-- 
2.39.5


^ permalink raw reply related

* [PATCH v2 0/2] x86/hyperv: MSI parent domain conversion
From: Nam Cao @ 2025-06-30 10:26 UTC (permalink / raw)
  To: K . Y . Srinivasan, Marc Zyngier, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H . Peter Anvin, linux-hyperv, linux-kernel
  Cc: Nam Cao

The initial implementation of PCI/MSI interrupt domains in the hierarchical
interrupt domain model used a shortcut by providing a global PCI/MSI
domain.

This works because the PCI/MSI[X] hardware is standardized and uniform, but
it violates the basic design principle of hierarchical interrupt domains:
Each hardware block involved in the interrupt delivery chain should have a
separate interrupt domain.

For PCI/MSI[X], the interrupt controller is per PCI device and not a global
made-up entity.

Unsurprisingly, the shortcut turned out to have downsides as it does not
allow dynamic allocation of interrupt vectors after initialization and it
prevents supporting IMS on PCI. For further details, see:

https://lore.kernel.org/lkml/20221111120501.026511281@linutronix.de/

The solution is implementing per device MSI domains, this means the
entities which provide global PCI/MSI domain so far have to implement MSI
parent domain functionality instead.

This series:

  - Fixup a dependency problem with CONFIG_GENERIC_MSI_IRQ

  - converts the x86 hyperv driver to implement MSI parent domain.

Changes in v2:

  - Add the first fixup patch to resolve a build failure

  - Add clarification to the TODO note.

 arch/x86/hyperv/irqdomain.c | 111 ++++++++++++++++++++++++------------
 drivers/hv/Kconfig          |   1 +
 drivers/irqchip/Kconfig     |   1 +
 3 files changed, 78 insertions(+), 35 deletions(-)

-- 
2.39.5


^ permalink raw reply

* Re: [PATCH 1/1] x86/hyperv: Switch to msi_create_parent_irq_domain()
From: kernel test robot @ 2025-06-28 15:18 UTC (permalink / raw)
  To: Nam Cao, K . Y . Srinivasan, Marc Zyngier, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, H . Peter Anvin, linux-hyperv, linux-kernel
  Cc: llvm, oe-kbuild-all, Nam Cao
In-Reply-To: <0eafade05acb51022242635750cd4990f3adb0ac.1750947640.git.namcao@linutronix.de>

Hi Nam,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/master]
[also build test WARNING on linus/master v6.16-rc3 next-20250627]
[cannot apply to tip/x86/core tip/auto-latest]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Nam-Cao/x86-hyperv-Switch-to-msi_create_parent_irq_domain/20250626-225420
base:   tip/master
patch link:    https://lore.kernel.org/r/0eafade05acb51022242635750cd4990f3adb0ac.1750947640.git.namcao%40linutronix.de
patch subject: [PATCH 1/1] x86/hyperv: Switch to msi_create_parent_irq_domain()
config: x86_64-buildonly-randconfig-002-20250628 (https://download.01.org/0day-ci/archive/20250628/202506282256.cHlEHrdc-lkp@intel.com/config)
compiler: clang version 20.1.7 (https://github.com/llvm/llvm-project 6146a88f60492b520a36f8f8f3231e15f3cc6082)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250628/202506282256.cHlEHrdc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506282256.cHlEHrdc-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/irqchip/irq-msi-lib.c:7:
>> include/linux/irqchip/irq-msi-lib.h:25:18: warning: declaration of 'struct msi_domain_info' will not be visible outside of this function [-Wvisibility]
      25 |                                struct msi_domain_info *info);
         |                                       ^
>> drivers/irqchip/irq-msi-lib.c:28:18: warning: declaration of 'struct msi_domain_info' will not be visible outside of this function [-Wvisibility]
      28 |                                struct msi_domain_info *info)
         |                                       ^
   drivers/irqchip/irq-msi-lib.c:26:6: error: conflicting types for 'msi_lib_init_dev_msi_info'
      26 | bool msi_lib_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
         |      ^
   include/linux/irqchip/irq-msi-lib.h:23:6: note: previous declaration is here
      23 | bool msi_lib_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
         |      ^
   drivers/irqchip/irq-msi-lib.c:30:51: error: no member named 'msi_parent_ops' in 'struct irq_domain'
      30 |         const struct msi_parent_ops *pops = real_parent->msi_parent_ops;
         |                                             ~~~~~~~~~~~  ^
   drivers/irqchip/irq-msi-lib.c:31:30: error: incomplete definition of type 'struct msi_domain_info'
      31 |         struct irq_chip *chip = info->chip;
         |                                 ~~~~^
   drivers/irqchip/irq-msi-lib.c:28:18: note: forward declaration of 'struct msi_domain_info'
      28 |                                struct msi_domain_info *info)
         |                                       ^
   drivers/irqchip/irq-msi-lib.c:43:31: error: incomplete definition of type 'const struct msi_parent_ops'
      43 |         if (domain->bus_token == pops->bus_select_token) {
         |                                  ~~~~^
   include/linux/irqdomain.h:27:8: note: forward declaration of 'struct msi_parent_ops'
      27 | struct msi_parent_ops;
         |        ^
   drivers/irqchip/irq-msi-lib.c:51:23: error: incomplete definition of type 'const struct msi_parent_ops'
      51 |         required_flags = pops->required_flags;
         |                          ~~~~^
   include/linux/irqdomain.h:27:8: note: forward declaration of 'struct msi_parent_ops'
      27 | struct msi_parent_ops;
         |        ^
   drivers/irqchip/irq-msi-lib.c:54:13: error: incomplete definition of type 'struct msi_domain_info'
      54 |         switch(info->bus_token) {
         |                ~~~~^
   drivers/irqchip/irq-msi-lib.c:28:18: note: forward declaration of 'struct msi_domain_info'
      28 |                                struct msi_domain_info *info)
         |                                       ^
   drivers/irqchip/irq-msi-lib.c:68:24: error: incomplete definition of type 'struct msi_domain_info'
      68 |                 if (WARN_ON_ONCE(info->flags))
         |                                  ~~~~^
   include/asm-generic/bug.h:117:25: note: expanded from macro 'WARN_ON_ONCE'
     117 |         int __ret_warn_on = !!(condition);                      \
         |                                ^~~~~~~~~
   drivers/irqchip/irq-msi-lib.c:28:18: note: forward declaration of 'struct msi_domain_info'
      28 |                                struct msi_domain_info *info)
         |                                       ^
   drivers/irqchip/irq-msi-lib.c:72:7: error: incomplete definition of type 'struct msi_domain_info'
      72 |                 info->flags = MSI_FLAG_ALLOC_SIMPLE_MSI_DESCS | MSI_FLAG_FREE_MSI_DESCS;
         |                 ~~~~^
   drivers/irqchip/irq-msi-lib.c:28:18: note: forward declaration of 'struct msi_domain_info'
      28 |                                struct msi_domain_info *info)
         |                                       ^
   drivers/irqchip/irq-msi-lib.c:72:17: error: use of undeclared identifier 'MSI_FLAG_ALLOC_SIMPLE_MSI_DESCS'
      72 |                 info->flags = MSI_FLAG_ALLOC_SIMPLE_MSI_DESCS | MSI_FLAG_FREE_MSI_DESCS;
         |                               ^
   drivers/irqchip/irq-msi-lib.c:72:51: error: use of undeclared identifier 'MSI_FLAG_FREE_MSI_DESCS'
      72 |                 info->flags = MSI_FLAG_ALLOC_SIMPLE_MSI_DESCS | MSI_FLAG_FREE_MSI_DESCS;
         |                                                                 ^
   drivers/irqchip/irq-msi-lib.c:76:22: error: use of undeclared identifier 'MSI_FLAG_PCI_MSI_MASK_PARENT'
      76 |                 required_flags &= ~MSI_FLAG_PCI_MSI_MASK_PARENT;
         |                                    ^
   drivers/irqchip/irq-msi-lib.c:91:6: error: incomplete definition of type 'struct msi_domain_info'
      91 |         info->flags                     &= pops->supported_flags;
         |         ~~~~^
   drivers/irqchip/irq-msi-lib.c:28:18: note: forward declaration of 'struct msi_domain_info'
      28 |                                struct msi_domain_info *info)
         |                                       ^
   drivers/irqchip/irq-msi-lib.c:91:23: error: incomplete definition of type 'const struct msi_parent_ops'
      91 |         info->flags                     &= pops->supported_flags;
         |                                            ~~~~^
   include/linux/irqdomain.h:27:8: note: forward declaration of 'struct msi_parent_ops'
      27 | struct msi_parent_ops;
         |        ^
   drivers/irqchip/irq-msi-lib.c:93:6: error: incomplete definition of type 'struct msi_domain_info'
      93 |         info->flags                     |= required_flags;
         |         ~~~~^
   drivers/irqchip/irq-msi-lib.c:28:18: note: forward declaration of 'struct msi_domain_info'
      28 |                                struct msi_domain_info *info)
         |                                       ^
   drivers/irqchip/irq-msi-lib.c:96:29: error: incomplete definition of type 'const struct msi_parent_ops'
      96 |         if (!chip->irq_eoi && (pops->chip_flags & MSI_CHIP_FLAG_SET_EOI))
         |                                ~~~~^
   include/linux/irqdomain.h:27:8: note: forward declaration of 'struct msi_parent_ops'
      27 | struct msi_parent_ops;
         |        ^
   drivers/irqchip/irq-msi-lib.c:96:44: error: use of undeclared identifier 'MSI_CHIP_FLAG_SET_EOI'
      96 |         if (!chip->irq_eoi && (pops->chip_flags & MSI_CHIP_FLAG_SET_EOI))
         |                                                   ^
   drivers/irqchip/irq-msi-lib.c:98:29: error: incomplete definition of type 'const struct msi_parent_ops'
      98 |         if (!chip->irq_ack && (pops->chip_flags & MSI_CHIP_FLAG_SET_ACK))
         |                                ~~~~^
   include/linux/irqdomain.h:27:8: note: forward declaration of 'struct msi_parent_ops'
      27 | struct msi_parent_ops;
         |        ^
   drivers/irqchip/irq-msi-lib.c:98:44: error: use of undeclared identifier 'MSI_CHIP_FLAG_SET_ACK'
      98 |         if (!chip->irq_ack && (pops->chip_flags & MSI_CHIP_FLAG_SET_ACK))
         |                                                   ^
   drivers/irqchip/irq-msi-lib.c:113:39: error: incomplete definition of type 'struct msi_domain_info'
     113 |         if (!chip->irq_set_affinity && !(info->flags & MSI_FLAG_NO_AFFINITY))
         |                                          ~~~~^
   drivers/irqchip/irq-msi-lib.c:28:18: note: forward declaration of 'struct msi_domain_info'
      28 |                                struct msi_domain_info *info)


vim +25 include/linux/irqchip/irq-msi-lib.h

496436f4a514a3 drivers/irqchip/irq-msi-lib.h Thomas Gleixner 2024-06-23  19  
72e257c6f05803 drivers/irqchip/irq-msi-lib.h Thomas Gleixner 2024-06-23  20  int msi_lib_irq_domain_select(struct irq_domain *d, struct irq_fwspec *fwspec,
72e257c6f05803 drivers/irqchip/irq-msi-lib.h Thomas Gleixner 2024-06-23  21  			      enum irq_domain_bus_token bus_token);
72e257c6f05803 drivers/irqchip/irq-msi-lib.h Thomas Gleixner 2024-06-23  22  
72e257c6f05803 drivers/irqchip/irq-msi-lib.h Thomas Gleixner 2024-06-23  23  bool msi_lib_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
72e257c6f05803 drivers/irqchip/irq-msi-lib.h Thomas Gleixner 2024-06-23  24  			       struct irq_domain *real_parent,
72e257c6f05803 drivers/irqchip/irq-msi-lib.h Thomas Gleixner 2024-06-23 @25  			       struct msi_domain_info *info);
72e257c6f05803 drivers/irqchip/irq-msi-lib.h Thomas Gleixner 2024-06-23  26  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* [PATCH v5 10/10] x86/hyperv/vtl: Use the wakeup mailbox to boot secondary CPUs
From: Ricardo Neri @ 2025-06-28  3:35 UTC (permalink / raw)
  To: x86, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
	K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Michael Kelley, Rafael J. Wysocki
  Cc: Saurabh Sengar, Chris Oo, Kirill A. Shutemov, linux-hyperv,
	devicetree, linux-acpi, linux-kernel, Ricardo Neri, Ricardo Neri
In-Reply-To: <20250627-rneri-wakeup-mailbox-v5-0-df547b1d196e@linux.intel.com>

The hypervisor is an untrusted entity for TDX guests. It cannot be used
to boot secondary CPUs. The function hv_vtl_wakeup_secondary_cpu() cannot
be used.

Instead, the virtual firmware boots the secondary CPUs and places them in
a state to transfer control to the kernel using the wakeup mailbox.

The kernel updates the APIC callback wakeup_secondary_cpu_64() to use
the mailbox if detected early during boot (enumerated via either an ACPI
table or a DeviceTree node).

Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v4:
 - None

Changes since v3:
 - Added Reviewed-by tag from Michael. Thanks!

Changes since v2:
 - Unconditionally use the wakeup mailbox in a TDX confidential VM.
   (Michael).
 - Edited the commit message for clarity.

Changes since v1:
 - None
---
 arch/x86/hyperv/hv_vtl.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index ed633a7e05b4..d82e040d9d9a 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -264,7 +264,15 @@ int __init hv_vtl_early_init(void)
 		panic("XSAVE has to be disabled as it is not supported by this module.\n"
 			  "Please add 'noxsave' to the kernel command line.\n");
 
-	apic_update_callback(wakeup_secondary_cpu_64, hv_vtl_wakeup_secondary_cpu);
+	/*
+	 * TDX confidential VMs do not trust the hypervisor and cannot use it to
+	 * boot secondary CPUs. Instead, they will be booted using the wakeup
+	 * mailbox if detected during boot. See setup_arch().
+	 *
+	 * There is no paravisor present if we are here.
+	 */
+	if (!hv_isolation_type_tdx())
+		apic_update_callback(wakeup_secondary_cpu_64, hv_vtl_wakeup_secondary_cpu);
 
 	return 0;
 }

-- 
2.43.0


^ permalink raw reply related

* [PATCH v5 09/10] x86/hyperv/vtl: Mark the wakeup mailbox page as private
From: Ricardo Neri @ 2025-06-28  3:35 UTC (permalink / raw)
  To: x86, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
	K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Michael Kelley, Rafael J. Wysocki
  Cc: Saurabh Sengar, Chris Oo, Kirill A. Shutemov, linux-hyperv,
	devicetree, linux-acpi, linux-kernel, Ricardo Neri, Yunhong Jiang,
	Ricardo Neri
In-Reply-To: <20250627-rneri-wakeup-mailbox-v5-0-df547b1d196e@linux.intel.com>

From: Yunhong Jiang <yunhong.jiang@linux.intel.com>

The current code maps MMIO devices as shared (decrypted) by default in a
confidential computing VM.

In a TDX environment, secondary CPUs are booted using the Multiprocessor
Wakeup Structure defined in the ACPI specification. The virtual firmware
and the operating system function in the guest context, without
intervention from the VMM. Map the physical memory of the mailbox as
private. Use the is_private_mmio() callback.

Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v4:
 - None

Changes since v3:
 - Updated to use the renamed function acpi_get_mp_wakeup_mailbox_paddr().
 - Added Reviewed-by tag from Michael. Thanks!

Changes since v2:
 - Use the new helper function get_mp_wakeup_mailbox_paddr().
 - Edited the commit message for clarity.

Changes since v1:
 - Added the helper function within_page() to improve readability
 - Override the is_private_mmio() callback when detecting a TDX
   environment. The address of the mailbox is checked in
   hv_is_private_mmio_tdx().
---
 arch/x86/hyperv/hv_vtl.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index ca0d23206e67..ed633a7e05b4 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -54,6 +54,18 @@ static void  __noreturn hv_vtl_restart(char __maybe_unused *cmd)
 	hv_vtl_emergency_restart();
 }
 
+static inline bool within_page(u64 addr, u64 start)
+{
+	return addr >= start && addr < (start + PAGE_SIZE);
+}
+
+static bool hv_vtl_is_private_mmio_tdx(u64 addr)
+{
+	u64 mb_addr = acpi_get_mp_wakeup_mailbox_paddr();
+
+	return mb_addr && within_page(addr, mb_addr);
+}
+
 void __init hv_vtl_init_platform(void)
 {
 	/*
@@ -66,6 +78,8 @@ void __init hv_vtl_init_platform(void)
 	/* There is no paravisor present if we are here. */
 	if (hv_isolation_type_tdx()) {
 		x86_init.resources.realmode_limit = SZ_4G;
+		x86_platform.hyper.is_private_mmio = hv_vtl_is_private_mmio_tdx;
+
 	} else {
 		x86_platform.realmode_reserve = x86_init_noop;
 		x86_platform.realmode_init = x86_init_noop;

-- 
2.43.0


^ permalink raw reply related

* [PATCH v5 08/10] x86/smpwakeup: Add a helper get the address of the wakeup mailbox
From: Ricardo Neri @ 2025-06-28  3:35 UTC (permalink / raw)
  To: x86, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
	K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Michael Kelley, Rafael J. Wysocki
  Cc: Saurabh Sengar, Chris Oo, Kirill A. Shutemov, linux-hyperv,
	devicetree, linux-acpi, linux-kernel, Ricardo Neri, Ricardo Neri
In-Reply-To: <20250627-rneri-wakeup-mailbox-v5-0-df547b1d196e@linux.intel.com>

A Hyper-V VTL level 2 guest on a TDX environment needs to map the
physical page of the ACPI Multiprocessor Wakeup Structure as private
(encrypted). It needs to know the physical address of this structure.
Add a helper function.

Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Suggested-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v4:
 - None

Changes since v3:
 - Renamed function to acpi_get_mp_wakeup_mailbox_paddr().
 - Added Reviewed-by tag from Michael. Thanks!

Changes since v2:
 - Introduced this patch

Changes since v1:
 - N/A
---
 arch/x86/include/asm/smp.h  | 1 +
 arch/x86/kernel/smpwakeup.c | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 77dce560a70a..158e8979342e 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -148,6 +148,7 @@ static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
 
 void acpi_setup_mp_wakeup_mailbox(u64 addr);
 struct acpi_madt_multiproc_wakeup_mailbox *acpi_get_mp_wakeup_mailbox(void);
+u64 acpi_get_mp_wakeup_mailbox_paddr(void);
 
 #else /* !CONFIG_SMP */
 #define wbinvd_on_cpu(cpu)     wbinvd()
diff --git a/arch/x86/kernel/smpwakeup.c b/arch/x86/kernel/smpwakeup.c
index 5089bcda615d..f730a66b6fc8 100644
--- a/arch/x86/kernel/smpwakeup.c
+++ b/arch/x86/kernel/smpwakeup.c
@@ -81,3 +81,8 @@ struct acpi_madt_multiproc_wakeup_mailbox *acpi_get_mp_wakeup_mailbox(void)
 {
 	return acpi_mp_wake_mailbox;
 }
+
+u64 acpi_get_mp_wakeup_mailbox_paddr(void)
+{
+	return acpi_mp_wake_mailbox_paddr;
+}

-- 
2.43.0


^ permalink raw reply related

* [PATCH v5 07/10] x86/hyperv/vtl: Setup the 64-bit trampoline for TDX guests
From: Ricardo Neri @ 2025-06-28  3:35 UTC (permalink / raw)
  To: x86, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
	K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Michael Kelley, Rafael J. Wysocki
  Cc: Saurabh Sengar, Chris Oo, Kirill A. Shutemov, linux-hyperv,
	devicetree, linux-acpi, linux-kernel, Ricardo Neri, Yunhong Jiang,
	Ricardo Neri
In-Reply-To: <20250627-rneri-wakeup-mailbox-v5-0-df547b1d196e@linux.intel.com>

From: Yunhong Jiang <yunhong.jiang@linux.intel.com>

The hypervisor is an untrusted entity for TDX guests. It cannot be used
to boot secondary CPUs - neither via hypercalls not the INIT assert,
de-assert plus Start-Up IPI messages.

Instead, the platform virtual firmware boots the secondary CPUs and
puts them in a state to transfer control to the kernel. This mechanism uses
the wakeup mailbox described in the Multiprocessor Wakeup Structure of the
ACPI specification. The entry point to the kernel is trampoline_start64.

Allocate and setup the trampoline using the default x86_platform callbacks.

The platform firmware configures the secondary CPUs in long mode. It is no
longer necessary to locate the trampoline under 1MB memory. After handoff
from firmware, the trampoline code switches briefly to 32-bit addressing
mode, which has an addressing limit of 4GB. Set the upper bound of the
trampoline memory accordingly.

Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
---
Changes since v4:
 - None

Changes since v3:
 - Added Reviewed-by tag from Michael. Thanks!

Changes since v2:
 - Added a note regarding there is no need to check for a present
   paravisor.
 - Edited commit message for clarity.

Changes since v1:
 - Dropped the function hv_reserve_real_mode(). Instead, used the new
   members realmode_limit and reserve_bios members of x86_init to
   set the upper bound of the trampoline memory. (Thomas)
---
 arch/x86/hyperv/hv_vtl.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index e10b63b7a49f..ca0d23206e67 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -63,9 +63,14 @@ void __init hv_vtl_init_platform(void)
 	 */
 	pr_info("Linux runs in Hyper-V Virtual Trust Level %d\n", ms_hyperv.vtl);
 
-	x86_platform.realmode_reserve = x86_init_noop;
-	x86_platform.realmode_init = x86_init_noop;
-	real_mode_header = &hv_vtl_real_mode_header;
+	/* There is no paravisor present if we are here. */
+	if (hv_isolation_type_tdx()) {
+		x86_init.resources.realmode_limit = SZ_4G;
+	} else {
+		x86_platform.realmode_reserve = x86_init_noop;
+		x86_platform.realmode_init = x86_init_noop;
+		real_mode_header = &hv_vtl_real_mode_header;
+	}
 	x86_init.irqs.pre_vector_init = x86_init_noop;
 	x86_init.timers.timer_init = x86_init_noop;
 	x86_init.resources.probe_roms = x86_init_noop;

-- 
2.43.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox