| Message ID | 20250207045354.27329-1-sarunkod@amd.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Emulated AMD IOMMU cleanup and fixes | expand |
Hi all, Accidentally added v2 tag. Please ignore it, this is version 1. On 2/7/2025 10:23 AM, Sairaj Kodilkar wrote: > This series provides few bug fixes for emulated AMD IOMMU. The series is based > on top of qemu upstream master commit d922088eb4ba. > > Patch 1: The code was using wrong DTE field to determine interrupt passthrough. > Hence replaced it with correct field according to [1]. > > Patch 2: Current code sets the PCI capability BAR low and high to the > lower and upper 16 bits of AMDVI_BASE_ADDR respectively, which is > wrong. Instead use 32 bit mask to set the PCI capability BAR low and > high. > The guest IOMMU driver works with current qemu code because it uses > base address from the IVRS table and not the one provided by > PCI capability. > > Sairaj Kodilkar (2): > amd_iommu: Use correct DTE field for interrupt passthrough > amd_iommu: Use correct bitmask to set capability BAR > > hw/i386/amd_iommu.c | 10 +++++----- > hw/i386/amd_iommu.h | 2 +- > 2 files changed, 6 insertions(+), 6 deletions(-) >
07.02.2025 07:53, Sairaj Kodilkar wrote: > This series provides few bug fixes for emulated AMD IOMMU. The series is based > on top of qemu upstream master commit d922088eb4ba. > > Patch 1: The code was using wrong DTE field to determine interrupt passthrough. > Hence replaced it with correct field according to [1]. > > Patch 2: Current code sets the PCI capability BAR low and high to the > lower and upper 16 bits of AMDVI_BASE_ADDR respectively, which is > wrong. Instead use 32 bit mask to set the PCI capability BAR low and > high. > The guest IOMMU driver works with current qemu code because it uses > base address from the IVRS table and not the one provided by > PCI capability. > > Sairaj Kodilkar (2): > amd_iommu: Use correct DTE field for interrupt passthrough > amd_iommu: Use correct bitmask to set capability BAR > > hw/i386/amd_iommu.c | 10 +++++----- > hw/i386/amd_iommu.h | 2 +- > 2 files changed, 6 insertions(+), 6 deletions(-) Is this qemu-stable material (current series: 7.2, 8.2, 9.2)? 3684717b74 "amd_iommu: Use correct bitmask to set capability BAR" does not apply to 7.2, since v8.0.0-10-g6291a28645 "hw/i386/amd_iommu: Explicit use of AMDVI_BASE_ADDR in amdvi_init" in not in 7.2, but the change can be adjusted for 7.2 easily, or 6291a28645 can be picked up too. Thanks, /mjt
Hi Michael, On 2/25/2025 2:17 PM, Michael Tokarev wrote: > 07.02.2025 07:53, Sairaj Kodilkar wrote: >> This series provides few bug fixes for emulated AMD IOMMU. The series is based >> on top of qemu upstream master commit d922088eb4ba. >> >> Patch 1: The code was using wrong DTE field to determine interrupt passthrough. >> Hence replaced it with correct field according to [1]. >> >> Patch 2: Current code sets the PCI capability BAR low and high to the >> lower and upper 16 bits of AMDVI_BASE_ADDR respectively, which is >> wrong. Instead use 32 bit mask to set the PCI capability BAR low and >> high. >> The guest IOMMU driver works with current qemu code because it uses >> base address from the IVRS table and not the one provided by >> PCI capability. >> >> Sairaj Kodilkar (2): >> amd_iommu: Use correct DTE field for interrupt passthrough >> amd_iommu: Use correct bitmask to set capability BAR >> >> hw/i386/amd_iommu.c | 10 +++++----- >> hw/i386/amd_iommu.h | 2 +- >> 2 files changed, 6 insertions(+), 6 deletions(-) > > Is this qemu-stable material (current series: 7.2, 8.2, 9.2)? Linux kernel doesn't use these changes. So its fine. But I believe we care for other OS as well? if yes then better to backport. > > 3684717b74 "amd_iommu: Use correct bitmask to set capability BAR" does > not apply to 7.2, since v8.0.0-10-g6291a28645 "hw/i386/amd_iommu: Explicit > use of AMDVI_BASE_ADDR in amdvi_init" in not in 7.2, but the change can be > adjusted for 7.2 easily, or 6291a28645 can be picked up too. How is this works? You will pick it up -OR- you want us to backport and send it to stable mailing list? -Vasant
26.02.2025 15:53, Vasant Hegde wrote: > Hi Michael, Hi! > On 2/25/2025 2:17 PM, Michael Tokarev wrote: ...>> Is this qemu-stable material (current series: 7.2, 8.2, 9.2)? > > Linux kernel doesn't use these changes. So its fine. But I believe we care for > other OS as well? if yes then better to backport. Yes, we definitely care about other OSes. There are numerous possible other questions though. For example, how relevant these changes are for older 7.2.x series, where AMD IOMMU is in less current state (missing all further development) so might not be as relevant anymore. >> 3684717b74 "amd_iommu: Use correct bitmask to set capability BAR" does >> not apply to 7.2, since v8.0.0-10-g6291a28645 "hw/i386/amd_iommu: Explicit >> use of AMDVI_BASE_ADDR in amdvi_init" in not in 7.2, but the change can be >> adjusted for 7.2 easily, or 6291a28645 can be picked up too. > > How is this works? You will pick it up -OR- you want us to backport and send it > to stable mailing list? This is just a data point, nothing more. Indicating that for 7.2, it needs some more work. I picked it up for 7.2 already: https://gitlab.com/mjt0k/qemu/-/tree/staging-7.2 But this is more mechanical way, maybe you, who know this area much better than me, prefer other way, like picking up already mentioned commit 6291a28645. Or maybe it isn't worth the effort for 7.2 anyway, provided the issue isn't that important and it needs any additional work to back-port. If you especially care about some older stable releases and think one or another change really needs to be there *and* needs some backporting work, you might do a backport yourself or give some notes for me to do that. It's always a trade-off between "importance" of the change, age of the stable series, the amount of work needed for backporting, and possibility of breakage. For less-important or less-used stuff, even thinking about this tradeoff is already too much work ;) Thanks, /mjt
Hi, On 2/26/2025 8:52 PM, Michael Tokarev wrote: > 26.02.2025 15:53, Vasant Hegde wrote: >> Hi Michael, > > Hi! > >> On 2/25/2025 2:17 PM, Michael Tokarev wrote: > ...>> Is this qemu-stable material (current series: 7.2, 8.2, 9.2)? >> >> Linux kernel doesn't use these changes. So its fine. But I believe we care for >> other OS as well? if yes then better to backport. > > Yes, we definitely care about other OSes. There are numerous possible > other questions though. For example, how relevant these changes are > for older 7.2.x series, where AMD IOMMU is in less current state (missing > all further development) so might not be as relevant anymore. > >>> 3684717b74 "amd_iommu: Use correct bitmask to set capability BAR" does >>> not apply to 7.2, since v8.0.0-10-g6291a28645 "hw/i386/amd_iommu: Explicit >>> use of AMDVI_BASE_ADDR in amdvi_init" in not in 7.2, but the change can be >>> adjusted for 7.2 easily, or 6291a28645 can be picked up too. >> >> How is this works? You will pick it up -OR- you want us to backport and send it >> to stable mailing list? > > This is just a data point, nothing more. Indicating that for 7.2, it needs some > more work. I picked it up for 7.2 already: https://gitlab.com/mjt0k/qemu/-/ > tree/staging-7.2 Thanks. Looks good. > But this is more mechanical way, maybe you, who know this area much better than > me, prefer other way, like picking up already mentioned commit 6291a28645. > Or maybe it isn't worth the effort for 7.2 anyway, provided the issue isn't > that important and it needs any additional work to back-port. > > If you especially care about some older stable releases and think one or > another change really needs to be there *and* needs some backporting work, > you might do a backport yourself or give some notes for me to do that. > > It's always a trade-off between "importance" of the change, age of the > stable series, the amount of work needed for backporting, and possibility > of breakage. For less-important or less-used stuff, even thinking about > this tradeoff is already too much work ;) :-) Thanks for detailed explanation. Next time, if its not applying cleanly we can backport it and give it to you. -Vasant
This series provides few bug fixes for emulated AMD IOMMU. The series is based on top of qemu upstream master commit d922088eb4ba. Patch 1: The code was using wrong DTE field to determine interrupt passthrough. Hence replaced it with correct field according to [1]. Patch 2: Current code sets the PCI capability BAR low and high to the lower and upper 16 bits of AMDVI_BASE_ADDR respectively, which is wrong. Instead use 32 bit mask to set the PCI capability BAR low and high. The guest IOMMU driver works with current qemu code because it uses base address from the IVRS table and not the one provided by PCI capability. Sairaj Kodilkar (2): amd_iommu: Use correct DTE field for interrupt passthrough amd_iommu: Use correct bitmask to set capability BAR hw/i386/amd_iommu.c | 10 +++++----- hw/i386/amd_iommu.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-)