Kay, Allen M
2011-Aug-22 22:01 UTC
[Xen-devel] RE: Resend: RE: enable_ats_device() call site
>And why is it VT-d specific then? The problem to solve is that enabling >may not happen when it is first attempted, in the case where Xen on its >own can''t be certain that using MMCFG is safe. Hence when the device >gets reported by Dom0 (or when MMCFG gets enabled for the respective >bus), another attempt needs to be made at enabling it. De-assigning and >then re-assigning the device to Dom0 seems to be overkill to me.The part that is VT-d specific is ATS capability reporting in ACPI DMAR table for reporting ATS capability in root ports. I not so clear on what do you mean by making another attempt when initial ATS enabling attempt fails. Do you have a patch to address this issue? Allen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2011-Aug-23 08:27 UTC
[Xen-devel] RE: Resend: RE: enable_ats_device() call site
>>> On 23.08.11 at 00:01, "Kay, Allen M" <allen.m.kay@intel.com> wrote: >> And why is it VT-d specific then? The problem to solve is that enabling >>may not happen when it is first attempted, in the case where Xen on its >>own can''t be certain that using MMCFG is safe. Hence when the device >>gets reported by Dom0 (or when MMCFG gets enabled for the respective >>bus), another attempt needs to be made at enabling it. De-assigning and >>then re-assigning the device to Dom0 seems to be overkill to me. > > The part that is VT-d specific is ATS capability reporting in ACPI DMAR > table for reporting ATS capability in root ports. I not so clear on what > do you mean by making another attempt when initial ATS enabling attempt > fails.The initial enabling may fail because of the unavailability of MMCFG accesses (due to the memory range(s) used not being reserved in E820, which is not required to be the case by the spec). Once MMCFG gets enabled, this needs to be re-attempted at some point, and the logical point appears to be when the device gets re-registered by Dom0.> Do you have a patch to address this issue?Only for the (trivial) ACS part so far (see below). There''s also a second aspect here: the bus-to-bridge mappings currently get established just once, during boot. This is already hot(un)plug incompatible, but will become even more of a problem when multi-segment support gets in (I should be sending out patches soon), for the same MMCFG-initially-unavailable reason as outlined above (as in that case non-zero segments can''t be scanned at boot). Jan --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -350,9 +350,10 @@ int pci_add_device(u16 seg, u8 bus, u8 d } list_add(&pdev->domain_list, &dom0->arch.pdev_list); - pci_enable_acs(pdev); } + pci_enable_acs(pdev); + out: spin_unlock(&pcidevs_lock); printk(XENLOG_DEBUG "PCI add %s %04x:%02x:%02x.%u\n", pdev_type, _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2011-Sep-20 13:56 UTC
[Xen-devel] RE: Resend: RE: enable_ats_device() call site
Ping?>>> On 23.08.11 at 10:27, "Jan Beulich" <JBeulich@novell.com> wrote: >>>> On 23.08.11 at 00:01, "Kay, Allen M" <allen.m.kay@intel.com> wrote: >>> And why is it VT-d specific then? The problem to solve is that enabling >>>may not happen when it is first attempted, in the case where Xen on its >>>own can''t be certain that using MMCFG is safe. Hence when the device >>>gets reported by Dom0 (or when MMCFG gets enabled for the respective >>>bus), another attempt needs to be made at enabling it. De-assigning and >>>then re-assigning the device to Dom0 seems to be overkill to me. >> >> The part that is VT-d specific is ATS capability reporting in ACPI DMAR >> table for reporting ATS capability in root ports. I not so clear on what >> do you mean by making another attempt when initial ATS enabling attempt >> fails. > > The initial enabling may fail because of the unavailability of MMCFG > accesses (due to the memory range(s) used not being reserved in E820, > which is not required to be the case by the spec). Once MMCFG gets > enabled, this needs to be re-attempted at some point, and the logical > point appears to be when the device gets re-registered by Dom0. > >> Do you have a patch to address this issue? > > Only for the (trivial) ACS part so far (see below). > > There''s also a second aspect here: the bus-to-bridge mappings > currently get established just once, during boot. This is already > hot(un)plug incompatible, but will become even more of a problem > when multi-segment support gets in (I should be sending out patches > soon), for the same MMCFG-initially-unavailable reason as outlined > above (as in that case non-zero segments can''t be scanned at boot). > > Jan > > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -350,9 +350,10 @@ int pci_add_device(u16 seg, u8 bus, u8 d > } > > list_add(&pdev->domain_list, &dom0->arch.pdev_list); > - pci_enable_acs(pdev); > } > > + pci_enable_acs(pdev); > + > out: > spin_unlock(&pcidevs_lock); > printk(XENLOG_DEBUG "PCI add %s %04x:%02x:%02x.%u\n", pdev_type, > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kay, Allen M
2011-Sep-20 18:02 UTC
RE: [Xen-devel] RE: Resend: RE: enable_ats_device() call site
Jan, Thanks for the reminder. This has slip through the cracks. The patch look good. With this patch, we shouldn''t need the existing call to pci_enable_acs() in setup_dom0_devices(), correct? I agree that the same problem exists for bus-to-bridge mapping function pci_scan_device(). By the way, we probably should take the opportunity to give it a proper name that reflects to the true purpose of this function. Allen -----Original Message----- From: Jan Beulich [mailto:JBeulich@suse.com] Sent: Tuesday, September 20, 2011 6:57 AM To: Kay, Allen M Cc: Xen Devel Subject: [Xen-devel] RE: Resend: RE: enable_ats_device() call site Ping?>>> On 23.08.11 at 10:27, "Jan Beulich" <JBeulich@novell.com> wrote: >>>> On 23.08.11 at 00:01, "Kay, Allen M" <allen.m.kay@intel.com> wrote: >>> And why is it VT-d specific then? The problem to solve is that enabling >>>may not happen when it is first attempted, in the case where Xen on its >>>own can''t be certain that using MMCFG is safe. Hence when the device >>>gets reported by Dom0 (or when MMCFG gets enabled for the respective >>>bus), another attempt needs to be made at enabling it. De-assigning and >>>then re-assigning the device to Dom0 seems to be overkill to me. >> >> The part that is VT-d specific is ATS capability reporting in ACPI DMAR >> table for reporting ATS capability in root ports. I not so clear on what >> do you mean by making another attempt when initial ATS enabling attempt >> fails. > > The initial enabling may fail because of the unavailability of MMCFG > accesses (due to the memory range(s) used not being reserved in E820, > which is not required to be the case by the spec). Once MMCFG gets > enabled, this needs to be re-attempted at some point, and the logical > point appears to be when the device gets re-registered by Dom0. > >> Do you have a patch to address this issue? > > Only for the (trivial) ACS part so far (see below). > > There''s also a second aspect here: the bus-to-bridge mappings > currently get established just once, during boot. This is already > hot(un)plug incompatible, but will become even more of a problem > when multi-segment support gets in (I should be sending out patches > soon), for the same MMCFG-initially-unavailable reason as outlined > above (as in that case non-zero segments can''t be scanned at boot). > > Jan > > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -350,9 +350,10 @@ int pci_add_device(u16 seg, u8 bus, u8 d > } > > list_add(&pdev->domain_list, &dom0->arch.pdev_list); > - pci_enable_acs(pdev); > } > > + pci_enable_acs(pdev); > + > out: > spin_unlock(&pcidevs_lock); > printk(XENLOG_DEBUG "PCI add %s %04x:%02x:%02x.%u\n", pdev_type, > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2011-Sep-21 06:45 UTC
RE: [Xen-devel] RE: Resend: RE: enable_ats_device() call site
>>> On 20.09.11 at 20:02, "Kay, Allen M" <allen.m.kay@intel.com> wrote: > Thanks for the reminder. This has slip through the cracks. The patch look > good. With this patch, we shouldn''t need the existing call to > pci_enable_acs() in setup_dom0_devices(), correct?Ah, indeed, I didn''t even pay attention to this. But you didn''t say anything about the subject of the mail, which is what keeps me from pushing the (incomplete) patch.> I agree that the same problem exists for bus-to-bridge mapping function > pci_scan_device(). By the way, we probably should take the opportunity to > give it a proper name that reflects to the true purpose of this function.It''s not that far off, if you mean scan_pci_devices() (and _scan_pci_devices() once the multi-seg patches get committed). What alternative name were you thinking of? Also, if the bus2bridge data got maintained dynamically (i.e. out of pci_{add,remove}_device()), I would think these functions could go away again (and hence renaming them would become mute). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel