* [PATCH] intel-iommu: fix Read DMAR IQA REG DW @ 2024-07-04 3:53 YeeLi 2024-07-04 6:54 ` Yee Li 2024-07-20 18:29 ` Michael S. Tsirkin 0 siblings, 2 replies; 8+ messages in thread From: YeeLi @ 2024-07-04 3:53 UTC (permalink / raw) To: mst, jasowang; +Cc: pbonzini, richard.henderson, eduardo, qemu-devel, yeeli From: yeeli <seven.yi.lee@gmail.com> When dmar_readq or devmem2 read the DW of IQA always 0UL because "& VTD_IQA_QS". So, try to fix it. case: after vtd_mem_write IQA val: 0x100206801 after vtd_mem_read IQA val: 0x100206001 Signed-off-by: yeeli <seven.yi.lee@gmail.com> --- hw/i386/intel_iommu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 37c21a0aec..e230a45940 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -2938,7 +2938,8 @@ static uint64_t vtd_mem_read(void *opaque, hwaddr addr, unsigned size) /* Invalidation Queue Address Register, 64-bit */ case DMAR_IQA_REG: - val = s->iq | (vtd_get_quad(s, DMAR_IQA_REG) & VTD_IQA_QS); + val = s->iq | (vtd_get_quad(s, DMAR_IQA_REG) & VTD_IQA_QS + | VTD_IQA_DW_MASK); if (size == 4) { val = val & ((1ULL << 32) - 1); } -- 2.34.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] intel-iommu: fix Read DMAR IQA REG DW 2024-07-04 3:53 [PATCH] intel-iommu: fix Read DMAR IQA REG DW YeeLi @ 2024-07-04 6:54 ` Yee Li 2024-07-20 18:28 ` Michael S. Tsirkin 2024-07-20 18:29 ` Michael S. Tsirkin 1 sibling, 1 reply; 8+ messages in thread From: Yee Li @ 2024-07-04 6:54 UTC (permalink / raw) To: mst, jasowang; +Cc: pbonzini, richard.henderson, eduardo, qemu-devel Sorry, the patch missing "(( ))" Replace "& VTD_IQA_QS" with "& (VTD_IQA_QS | VTD_IQA_DW_MASK))" is correct. Revised patch as follows, Signed-off-by: yeeli <seven.yi.lee@gmail.com> --- hw/i386/intel_iommu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 37c21a0aec..23562ba26b 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -2938,7 +2938,8 @@ static uint64_t vtd_mem_read(void *opaque, hwaddr addr, unsigned size) /* Invalidation Queue Address Register, 64-bit */ case DMAR_IQA_REG: - val = s->iq | (vtd_get_quad(s, DMAR_IQA_REG) & VTD_IQA_QS); + val = s->iq | (vtd_get_quad(s, DMAR_IQA_REG) & (VTD_IQA_QS + | VTD_IQA_DW_MASK)); if (size == 4) { val = val & ((1ULL << 32) - 1); } -- 2.34.1 YeeLi <seven.yi.lee@gmail.com> 于2024年7月4日周四 11:53写道： > > From: yeeli <seven.yi.lee@gmail.com> > > When dmar_readq or devmem2 read the DW of IQA always 0UL because > "& VTD_IQA_QS". So, try to fix it. > > case: > after vtd_mem_write > IQA val: 0x100206801 > > after vtd_mem_read > IQA val: 0x100206001 > > Signed-off-by: yeeli <seven.yi.lee@gmail.com> > --- > hw/i386/intel_iommu.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 37c21a0aec..e230a45940 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -2938,7 +2938,8 @@ static uint64_t vtd_mem_read(void *opaque, hwaddr addr, unsigned size) > > /* Invalidation Queue Address Register, 64-bit */ > case DMAR_IQA_REG: > - val = s->iq | (vtd_get_quad(s, DMAR_IQA_REG) & VTD_IQA_QS); > + val = s->iq | (vtd_get_quad(s, DMAR_IQA_REG) & VTD_IQA_QS > + | VTD_IQA_DW_MASK); > if (size == 4) { > val = val & ((1ULL << 32) - 1); > } > -- > 2.34.1 > ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] intel-iommu: fix Read DMAR IQA REG DW 2024-07-04 6:54 ` Yee Li @ 2024-07-20 18:28 ` Michael S. Tsirkin 0 siblings, 0 replies; 8+ messages in thread From: Michael S. Tsirkin @ 2024-07-20 18:28 UTC (permalink / raw) To: Yee Li; +Cc: jasowang, pbonzini, richard.henderson, eduardo, qemu-devel On Thu, Jul 04, 2024 at 02:54:00PM +0800, Yee Li wrote: > Sorry, the patch missing "(( ))" > Replace "& VTD_IQA_QS" with "& (VTD_IQA_QS | VTD_IQA_DW_MASK))" is correct. > Revised patch as follows, > So submit it properly. Also, how did you test the patch? > Signed-off-by: yeeli <seven.yi.lee@gmail.com> > --- > hw/i386/intel_iommu.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 37c21a0aec..23562ba26b 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -2938,7 +2938,8 @@ static uint64_t vtd_mem_read(void *opaque, > hwaddr addr, unsigned size) > > /* Invalidation Queue Address Register, 64-bit */ > case DMAR_IQA_REG: > - val = s->iq | (vtd_get_quad(s, DMAR_IQA_REG) & VTD_IQA_QS); > + val = s->iq | (vtd_get_quad(s, DMAR_IQA_REG) & (VTD_IQA_QS > + | VTD_IQA_DW_MASK)); This is a very messy way to write this. Align things properly pls. > if (size == 4) { > val = val & ((1ULL << 32) - 1); > } > -- > 2.34.1 > > YeeLi <seven.yi.lee@gmail.com> 于2024年7月4日周四 11:53写道： > > > > From: yeeli <seven.yi.lee@gmail.com> > > > > When dmar_readq or devmem2 read the DW of IQA always 0UL because > > "& VTD_IQA_QS". So, try to fix it. > > > > case: > > after vtd_mem_write > > IQA val: 0x100206801 > > > > after vtd_mem_read > > IQA val: 0x100206001 > > > > Signed-off-by: yeeli <seven.yi.lee@gmail.com> > > --- > > hw/i386/intel_iommu.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > > index 37c21a0aec..e230a45940 100644 > > --- a/hw/i386/intel_iommu.c > > +++ b/hw/i386/intel_iommu.c > > @@ -2938,7 +2938,8 @@ static uint64_t vtd_mem_read(void *opaque, hwaddr addr, unsigned size) > > > > /* Invalidation Queue Address Register, 64-bit */ > > case DMAR_IQA_REG: > > - val = s->iq | (vtd_get_quad(s, DMAR_IQA_REG) & VTD_IQA_QS); > > + val = s->iq | (vtd_get_quad(s, DMAR_IQA_REG) & VTD_IQA_QS > > + | VTD_IQA_DW_MASK); > > if (size == 4) { > > val = val & ((1ULL << 32) - 1); > > } > > -- > > 2.34.1 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] intel-iommu: fix Read DMAR IQA REG DW 2024-07-04 3:53 [PATCH] intel-iommu: fix Read DMAR IQA REG DW YeeLi 2024-07-04 6:54 ` Yee Li @ 2024-07-20 18:29 ` Michael S. Tsirkin 2024-07-23 2:40 ` Yee Li 1 sibling, 1 reply; 8+ messages in thread From: Michael S. Tsirkin @ 2024-07-20 18:29 UTC (permalink / raw) To: YeeLi; +Cc: jasowang, pbonzini, richard.henderson, eduardo, qemu-devel On Thu, Jul 04, 2024 at 11:53:02AM +0800, YeeLi wrote: > From: yeeli <seven.yi.lee@gmail.com> > > When dmar_readq or devmem2 read the DW of IQA always 0UL because > "& VTD_IQA_QS". So, try to fix it. > > case: > after vtd_mem_write > IQA val: 0x100206801 > > after vtd_mem_read > IQA val: 0x100206001 > > Signed-off-by: yeeli <seven.yi.lee@gmail.com> how was this tested? > --- > hw/i386/intel_iommu.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 37c21a0aec..e230a45940 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -2938,7 +2938,8 @@ static uint64_t vtd_mem_read(void *opaque, hwaddr addr, unsigned size) > > /* Invalidation Queue Address Register, 64-bit */ > case DMAR_IQA_REG: > - val = s->iq | (vtd_get_quad(s, DMAR_IQA_REG) & VTD_IQA_QS); > + val = s->iq | (vtd_get_quad(s, DMAR_IQA_REG) & VTD_IQA_QS > + | VTD_IQA_DW_MASK); > if (size == 4) { > val = val & ((1ULL << 32) - 1); > } > -- > 2.34.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] intel-iommu: fix Read DMAR IQA REG DW 2024-07-20 18:29 ` Michael S. Tsirkin @ 2024-07-23 2:40 ` Yee Li 2024-07-23 3:02 ` Yee Li 2024-07-23 8:05 ` Yi Liu 0 siblings, 2 replies; 8+ messages in thread From: Yee Li @ 2024-07-23 2:40 UTC (permalink / raw) To: Michael S. Tsirkin Cc: jasowang, pbonzini, richard.henderson, eduardo, qemu-devel > > When dmar_readq or devmem2 read the DW of IQA always 0UL because > > "& VTD_IQA_QS". So, try to fix it. > > > > case: > > after vtd_mem_write > > IQA val: 0x100206801 > > > > after vtd_mem_read > > IQA val: 0x100206001 > > > > Signed-off-by: yeeli <seven.yi.lee@gmail.com> > > > how was this tested? If VT-D hardware supports scalable mode, Linux will set the IQA DW (bit 11). In qemu, the vtd_mem_write and vtd_update_iq_dw set DW well, however, vtd_mem_read the DW wrong because "& VTD_IQA_QS" dropped the value of DW (bit 11). So testing the patch is easy, config the "x-scalable-mode" option as below: "-device intel-iommu,caching-mode=on,x-scalable-mode=on,aw-bits=48" after Linux OS boot, check the IQA_REG DW (bit 11) as below: IOMMU_DEBUGFS: "cat /sys/kernel/debug/iommu/intel |grep IQA" "IQA 0x90 0x00000001001da801" or devmem2 read the reg: "devmem2 0xfed90090" "/dev/mem opened." "Memory mapped at address 0x7f983014f000." "Value at address 0xFED90090 (0x7f983014f000): 0x0" ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] intel-iommu: fix Read DMAR IQA REG DW 2024-07-23 2:40 ` Yee Li @ 2024-07-23 3:02 ` Yee Li 2024-07-23 8:05 ` Yi Liu 1 sibling, 0 replies; 8+ messages in thread From: Yee Li @ 2024-07-23 3:02 UTC (permalink / raw) To: Michael S. Tsirkin Cc: jasowang, pbonzini, richard.henderson, eduardo, qemu-devel > or devmem2 read the reg: > > "devmem2 0xfed90090" > "/dev/mem opened." > "Memory mapped at address 0x7f983014f000." > "Value at address 0xFED90090 (0x7f983014f000): 0x0" Sorry, correct the devmem2 read value. "Value at address 0xFED90090 (0x7f983014f000): 0x1DA801" ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] intel-iommu: fix Read DMAR IQA REG DW 2024-07-23 2:40 ` Yee Li 2024-07-23 3:02 ` Yee Li @ 2024-07-23 8:05 ` Yi Liu 2024-07-24 6:34 ` Yee Li 1 sibling, 1 reply; 8+ messages in thread From: Yi Liu @ 2024-07-23 8:05 UTC (permalink / raw) To: Yee Li, Michael S. Tsirkin Cc: jasowang, pbonzini, richard.henderson, eduardo, qemu-devel On 2024/7/23 10:40, Yee Li wrote: >>> When dmar_readq or devmem2 read the DW of IQA always 0UL because >>> "& VTD_IQA_QS". So, try to fix it. >>> >>> case: >>> after vtd_mem_write >>> IQA val: 0x100206801 >>> >>> after vtd_mem_read >>> IQA val: 0x100206001 >>> >>> Signed-off-by: yeeli <seven.yi.lee@gmail.com> I think you may need to capitalize the first character of your first and last name. >> >> how was this tested? > > If VT-D hardware supports scalable mode, Linux will set the IQA DW (bit 11). > In qemu, the vtd_mem_write and vtd_update_iq_dw set DW well, however, > vtd_mem_read the DW wrong because "& VTD_IQA_QS" dropped the value > of DW (bit 11). > > So testing the patch is easy, > > config the "x-scalable-mode" option as below: > > "-device intel-iommu,caching-mode=on,x-scalable-mode=on,aw-bits=48" > > after Linux OS boot, check the IQA_REG DW (bit 11) as below: > > IOMMU_DEBUGFS: > > "cat /sys/kernel/debug/iommu/intel |grep IQA" > "IQA 0x90 0x00000001001da801" > > or devmem2 read the reg: > > "devmem2 0xfed90090" > "/dev/mem opened." > "Memory mapped at address 0x7f983014f000." > "Value at address 0xFED90090 (0x7f983014f000): 0x0" > So you found it by checking the debugfs output, and it looks to miss the DW bit. is it? Put a clearer commit message would be helpful. Please address Michael's comment, add a "Fixes: xxx" tag and resend. -- Regards, Yi Liu ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] intel-iommu: fix Read DMAR IQA REG DW 2024-07-23 8:05 ` Yi Liu @ 2024-07-24 6:34 ` Yee Li 0 siblings, 0 replies; 8+ messages in thread From: Yee Li @ 2024-07-24 6:34 UTC (permalink / raw) To: Yi Liu Cc: Michael S. Tsirkin, jasowang, pbonzini, richard.henderson, eduardo, qemu-devel > So you found it by checking the debugfs output, and it looks to miss > the DW bit. is it? Put a clearer commit message would be helpful. Yes, it is. So, "dropped the value of DW field" is indeed a bug? > Please address Michael's comment, add a "Fixes: xxx" tag and resend. OK, I will. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-07-24 6:35 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-04 3:53 [PATCH] intel-iommu: fix Read DMAR IQA REG DW YeeLi 2024-07-04 6:54 ` Yee Li 2024-07-20 18:28 ` Michael S. Tsirkin 2024-07-20 18:29 ` Michael S. Tsirkin 2024-07-23 2:40 ` Yee Li 2024-07-23 3:02 ` Yee Li 2024-07-23 8:05 ` Yi Liu 2024-07-24 6:34 ` Yee Li
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).