* [PATCH] intel_iommu: Allow both Status Write and Interrupt Flag in QI wait
@ 2025-07-11 9:47 David Woodhouse
2025-07-13 21:29 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: David Woodhouse @ 2025-07-11 9:47 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang, Le Tan, kib
Cc: Yi Liu, Clément Mathieu--Drif, Marcel Apfelbaum,
Paolo Bonzini, Richard Henderson, Eduardo Habkost, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2883 bytes --]
From: David Woodhouse <dwmw@amazon.co.uk>
FreeBSD does both, and this appears to be perfectly valid. The VT-d
spec even talks about the ordering (the status write should be done
first, unsurprisingly).
We certainly shouldn't assert() and abort QEMU if the guest asks for
both.
Fixes: ed7b8fbcfb88 ("intel-iommu: add supports for queued invalidation interface")
Closes: https://gitlab.com/qemu-project/qemu/-/issues/3028
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
Still can't get FreeBSD to boot and use CPUs with APIC ID > 255 using
*either* Intel or AMD IOMMU with interrupt remapping, or the native
15-bit APIC ID enlightenment.
cf. https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=288122
hw/i386/intel_iommu.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 69d72ad35c..3bfaf9bd07 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2822,6 +2822,7 @@ static bool vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
{
uint64_t mask[4] = {VTD_INV_DESC_WAIT_RSVD_LO, VTD_INV_DESC_WAIT_RSVD_HI,
VTD_INV_DESC_ALL_ONE, VTD_INV_DESC_ALL_ONE};
+ bool ret = true;
if (!vtd_inv_desc_reserved_check(s, inv_desc, mask, false,
__func__, "wait")) {
@@ -2833,7 +2834,8 @@ static bool vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
uint32_t status_data = (uint32_t)(inv_desc->lo >>
VTD_INV_DESC_WAIT_DATA_SHIFT);
- assert(!(inv_desc->lo & VTD_INV_DESC_WAIT_IF));
+ if (inv_desc->lo & VTD_INV_DESC_WAIT_IF)
+ vtd_generate_completion_event(s);
/* FIXME: need to be masked with HAW? */
dma_addr_t status_addr = inv_desc->hi;
@@ -2843,18 +2845,22 @@ static bool vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
&status_data, sizeof(status_data),
MEMTXATTRS_UNSPECIFIED)) {
trace_vtd_inv_desc_wait_write_fail(inv_desc->hi, inv_desc->lo);
- return false;
+ ret = false;
}
- } else if (inv_desc->lo & VTD_INV_DESC_WAIT_IF) {
+ }
+
+ if (inv_desc->lo & VTD_INV_DESC_WAIT_IF) {
/* Interrupt flag */
vtd_generate_completion_event(s);
- } else {
+ }
+
+ if (!(inv_desc->lo & (VTD_INV_DESC_WAIT_IF|VTD_INV_DESC_WAIT_SW))) {
error_report_once("%s: invalid wait desc: hi=%"PRIx64", lo=%"PRIx64
" (unknown type)", __func__, inv_desc->hi,
inv_desc->lo);
return false;
}
- return true;
+ return ret;
}
static bool vtd_process_context_cache_desc(IntelIOMMUState *s,
--
2.43.0
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] intel_iommu: Allow both Status Write and Interrupt Flag in QI wait
2025-07-11 9:47 [PATCH] intel_iommu: Allow both Status Write and Interrupt Flag in QI wait David Woodhouse
@ 2025-07-13 21:29 ` Michael S. Tsirkin
2025-07-14 7:50 ` David Woodhouse
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2025-07-13 21:29 UTC (permalink / raw)
To: David Woodhouse
Cc: Jason Wang, Le Tan, kib, Yi Liu, Clément Mathieu--Drif,
Marcel Apfelbaum, Paolo Bonzini, Richard Henderson,
Eduardo Habkost, qemu-devel
On Fri, Jul 11, 2025 at 10:47:47AM +0100, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> FreeBSD does both, and this appears to be perfectly valid. The VT-d
> spec even talks about the ordering (the status write should be done
> first, unsurprisingly).
>
> We certainly shouldn't assert() and abort QEMU if the guest asks for
> both.
>
> Fixes: ed7b8fbcfb88 ("intel-iommu: add supports for queued invalidation interface")
> Closes: https://gitlab.com/qemu-project/qemu/-/issues/3028
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
> Still can't get FreeBSD to boot and use CPUs with APIC ID > 255 using
> *either* Intel or AMD IOMMU with interrupt remapping, or the native
> 15-bit APIC ID enlightenment.
> cf. https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=288122
>
> hw/i386/intel_iommu.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 69d72ad35c..3bfaf9bd07 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2822,6 +2822,7 @@ static bool vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
> {
> uint64_t mask[4] = {VTD_INV_DESC_WAIT_RSVD_LO, VTD_INV_DESC_WAIT_RSVD_HI,
> VTD_INV_DESC_ALL_ONE, VTD_INV_DESC_ALL_ONE};
> + bool ret = true;
>
> if (!vtd_inv_desc_reserved_check(s, inv_desc, mask, false,
> __func__, "wait")) {
> @@ -2833,7 +2834,8 @@ static bool vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
> uint32_t status_data = (uint32_t)(inv_desc->lo >>
> VTD_INV_DESC_WAIT_DATA_SHIFT);
>
> - assert(!(inv_desc->lo & VTD_INV_DESC_WAIT_IF));
> + if (inv_desc->lo & VTD_INV_DESC_WAIT_IF)
> + vtd_generate_completion_event(s);
>
> /* FIXME: need to be masked with HAW? */
> dma_addr_t status_addr = inv_desc->hi;
Follow QEMU coding style, please.
> @@ -2843,18 +2845,22 @@ static bool vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
> &status_data, sizeof(status_data),
> MEMTXATTRS_UNSPECIFIED)) {
> trace_vtd_inv_desc_wait_write_fail(inv_desc->hi, inv_desc->lo);
> - return false;
> + ret = false;
> }
> - } else if (inv_desc->lo & VTD_INV_DESC_WAIT_IF) {
> + }
> +
> + if (inv_desc->lo & VTD_INV_DESC_WAIT_IF) {
> /* Interrupt flag */
> vtd_generate_completion_event(s);
> - } else {
> + }
> +
> + if (!(inv_desc->lo & (VTD_INV_DESC_WAIT_IF|VTD_INV_DESC_WAIT_SW))) {
> error_report_once("%s: invalid wait desc: hi=%"PRIx64", lo=%"PRIx64
> " (unknown type)", __func__, inv_desc->hi,
> inv_desc->lo);
> return false;
> }
> - return true;
> + return ret;
> }
>
> static bool vtd_process_context_cache_desc(IntelIOMMUState *s,
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] intel_iommu: Allow both Status Write and Interrupt Flag in QI wait
2025-07-13 21:29 ` Michael S. Tsirkin
@ 2025-07-14 7:50 ` David Woodhouse
0 siblings, 0 replies; 3+ messages in thread
From: David Woodhouse @ 2025-07-14 7:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, Le Tan, kib, Yi Liu, Clément Mathieu--Drif,
Marcel Apfelbaum, Paolo Bonzini, Richard Henderson,
Eduardo Habkost, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 515 bytes --]
On Sun, 2025-07-13 at 17:29 -0400, Michael S. Tsirkin wrote:
>
> > + if (inv_desc->lo & VTD_INV_DESC_WAIT_IF)
> > + vtd_generate_completion_event(s);
> >
> > /* FIXME: need to be masked with HAW? */
> > dma_addr_t status_addr = inv_desc->hi;
>
> Follow QEMU coding style, please.
Hm, actually that one doesn't need to be there at all, since it's done
a few lines later (*after* the status write). I'll send v2; thanks.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-07-14 8:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-11 9:47 [PATCH] intel_iommu: Allow both Status Write and Interrupt Flag in QI wait David Woodhouse
2025-07-13 21:29 ` Michael S. Tsirkin
2025-07-14 7:50 ` David Woodhouse
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).