| Message ID | 20190723154851.77627-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/iommu: fixes to hwdom_iommu_map | expand |
On 23.07.2019 17:48, Roger Pau Monne wrote: > Current code only prevent mapping the lapic page into the guest > physical memory map. Expand the range to be 0xFEEx_xxxx as described > in the Intel VTd specification section 3.13 "Handling Requests to > Interrupt Address Range". Right, and it being device side accesses that are of interest here, the LAPIC address range (visible to the CPU only) shouldn't even matter. Hence after some back and forth with myself I agree that you remove the entire comment there. > AMD also lists this address range in the AMD SR5690 Databook, section > 2.4.4 "MSI Interrupt Handling and MSI to HT Interrupt Conversion". Which raises the question about yet another patch to also exclude the HT range. > Requested-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
On 23.07.2019 17:48, Roger Pau Monne wrote: > Current code only prevent mapping the lapic page into the guest > physical memory map. Expand the range to be 0xFEEx_xxxx as described > in the Intel VTd specification section 3.13 "Handling Requests to > Interrupt Address Range". > > AMD also lists this address range in the AMD SR5690 Databook, section > 2.4.4 "MSI Interrupt Handling and MSI to HT Interrupt Conversion". > > Requested-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> I've committed this on the basis that it shouldn't hurt, but having thought about this some more I'm not really sure I see the point: The IOMMU special cases accesses into this range anyway, to redirect lookup to the interrupt remapping table instead of the DMA remapping one. Hence any mappings inserted into this range are simply useless, but shouldn't otherwise hurt. Jan
On Thu, Jul 25, 2019 at 10:22:17AM +0000, Jan Beulich wrote: > On 23.07.2019 17:48, Roger Pau Monne wrote: > > Current code only prevent mapping the lapic page into the guest > > physical memory map. Expand the range to be 0xFEEx_xxxx as described > > in the Intel VTd specification section 3.13 "Handling Requests to > > Interrupt Address Range". > > > > AMD also lists this address range in the AMD SR5690 Databook, section > > 2.4.4 "MSI Interrupt Handling and MSI to HT Interrupt Conversion". > > > > Requested-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > I've committed this on the basis that it shouldn't hurt, but having > thought about this some more I'm not really sure I see the point: > The IOMMU special cases accesses into this range anyway, to redirect > lookup to the interrupt remapping table instead of the DMA remapping > one. Hence any mappings inserted into this range are simply useless, > but shouldn't otherwise hurt. Intel SDM contains: "Software must ensure the second-level paging-structure entries are programmed not to remap input addresses to the interrupt address range. Hardware behavior is undefined for memory requests remapped to the interrupt address range." In section 3.13 (Handling Requests to Interrupt Address Range). Since arch_iommu_hwdom_init/hwdom_iommu_map adds entries to both the hap and the iommu page tables (or to hap only if shared) Xen should be careful to not map this range because the iommu special cases this range, but I'm not sure what hap does. Thanks, Roger.
> -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Roger Pau Monné > Sent: 25 July 2019 11:54 > To: Jan Beulich <JBeulich@suse.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; xen-devel@lists.xenproject.org > Subject: Re: [Xen-devel] [PATCH 1/2] x86/iommu: avoid mapping the interrupt address range for hwdom > > On Thu, Jul 25, 2019 at 10:22:17AM +0000, Jan Beulich wrote: > > On 23.07.2019 17:48, Roger Pau Monne wrote: > > > Current code only prevent mapping the lapic page into the guest > > > physical memory map. Expand the range to be 0xFEEx_xxxx as described > > > in the Intel VTd specification section 3.13 "Handling Requests to > > > Interrupt Address Range". > > > > > > AMD also lists this address range in the AMD SR5690 Databook, section > > > 2.4.4 "MSI Interrupt Handling and MSI to HT Interrupt Conversion". > > > > > > Requested-by: Andrew Cooper <andrew.cooper3@citrix.com> > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > > > I've committed this on the basis that it shouldn't hurt, but having > > thought about this some more I'm not really sure I see the point: > > The IOMMU special cases accesses into this range anyway, to redirect > > lookup to the interrupt remapping table instead of the DMA remapping > > one. Hence any mappings inserted into this range are simply useless, > > but shouldn't otherwise hurt. > > Intel SDM contains: > > "Software must ensure the second-level paging-structure entries are > programmed not to remap input addresses to the interrupt address > range. Hardware behavior is undefined for memory requests remapped to > the interrupt address range." > > In section 3.13 (Handling Requests to Interrupt Address Range). > > Since arch_iommu_hwdom_init/hwdom_iommu_map adds entries to both the > hap and the iommu page tables (or to hap only if shared) Xen should be > careful to not map this range because the iommu special cases this > range, but I'm not sure what hap does. Presumably such ranges should never end up in the P2M as they'd need to be trapped for emulation? Paul > > Thanks, Roger. > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel
On 25.07.2019 12:54, Roger Pau Monné wrote: > On Thu, Jul 25, 2019 at 10:22:17AM +0000, Jan Beulich wrote: >> On 23.07.2019 17:48, Roger Pau Monne wrote: >>> Current code only prevent mapping the lapic page into the guest >>> physical memory map. Expand the range to be 0xFEEx_xxxx as described >>> in the Intel VTd specification section 3.13 "Handling Requests to >>> Interrupt Address Range". >>> >>> AMD also lists this address range in the AMD SR5690 Databook, section >>> 2.4.4 "MSI Interrupt Handling and MSI to HT Interrupt Conversion". >>> >>> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >> >> I've committed this on the basis that it shouldn't hurt, but having >> thought about this some more I'm not really sure I see the point: >> The IOMMU special cases accesses into this range anyway, to redirect >> lookup to the interrupt remapping table instead of the DMA remapping >> one. Hence any mappings inserted into this range are simply useless, >> but shouldn't otherwise hurt. > > Intel SDM contains: > > "Software must ensure the second-level paging-structure entries are > programmed not to remap input addresses to the interrupt address > range. Hardware behavior is undefined for memory requests remapped to > the interrupt address range." > > In section 3.13 (Handling Requests to Interrupt Address Range). > > Since arch_iommu_hwdom_init/hwdom_iommu_map adds entries to both the > hap and the iommu page tables (or to hap only if shared) Xen should be > careful to not map this range because the iommu special cases this > range, but I'm not sure what hap does. Hmm, in the shared-pt case this is indeed desirable in any event. I have to admit that I didn't recall that even in the non-shared case arch_iommu_hwdom_init() would fiddle with the HAP tables as well, rather than just the IOMMU ones. I don't think this should remain this way long term. Or was there a reason for this to be needed? Jan
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c index 0fa6dcc3fd..88a87cf7a4 100644 --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -226,19 +226,9 @@ static bool __hwdom_init hwdom_iommu_map(const struct domain *d, return false; } - /* - * Check that it doesn't overlap with the LAPIC - * TODO: if the guest relocates the MMIO area of the LAPIC Xen should make - * sure there's nothing in the new address that would prevent trapping. - */ - if ( has_vlapic(d) ) - { - const struct vcpu *v; - - for_each_vcpu(d, v) - if ( pfn == PFN_DOWN(vlapic_base_address(vcpu_vlapic(v))) ) - return false; - } + /* Check that it doesn't overlap with the Interrupt Address Range. */ + if ( pfn >= 0xfee00 && pfn <= 0xfeeff ) + return false; /* ... or the IO-APIC */ for ( i = 0; has_vioapic(d) && i < d->arch.hvm.nr_vioapics; i++ ) if ( pfn == PFN_DOWN(domain_vioapic(d, i)->base_address) )
Current code only prevent mapping the lapic page into the guest physical memory map. Expand the range to be 0xFEEx_xxxx as described in the Intel VTd specification section 3.13 "Handling Requests to Interrupt Address Range". AMD also lists this address range in the AMD SR5690 Databook, section 2.4.4 "MSI Interrupt Handling and MSI to HT Interrupt Conversion". Requested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/drivers/passthrough/x86/iommu.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-)