enable_ats_device() call site

enable_ats_device() call site

[Jan Beulich]

Allen,

what is the reason for calling this from VT-d's domain_context_mapping()?
I neither undertsand why this is VT-d specific, nor why it needs to be
re-done with each device re-assignment.

I'm asking because this depends on MMCFG availability, and hence the
initial call from pci_add_device() (in the context of scan_pci_devices())
to iommu_add_device() may not result in this getting enabled, while on
the first Dom0-invoked pci_add_device() pdev->domain is already set
and hence iommu_add_device() doesn't get called at all. I'd therefore
like to pull this out into pci_add_device() (and call it, together with
pci_enable_acs(), after the conditional around iommu_add_device() -
should be safe as I view both enabling functions as idempotent).

Alternatively - why do we need scan_pci_devices() at all? We're
supposed to be getting the devices reported from Dom0 anyway.

Jan

RE: enable_ats_device() call site

[Kay, Allen M]

> what is the reason for calling this from VT-d's domain_context_mapping()?
> I neither undertsand why this is VT-d specific, nor why it needs to be
> re-done with each device re-assignment.

The reason is FLR clears the ATS enabled bit so we need to re-enable it for every re-assignment.  The reason we don't need to do this for ACS might be ACS reside on the bridge, not in the PCI endpoint.  ATS on the other hand, resides in PCI endpoints.

> Alternatively - why do we need scan_pci_devices() at all? We're
> supposed to be getting the devices reported from Dom0 anyway

Looks like it is use for building bus2bridge[] which is used for figuring out upstream bridges which are needed when assigning non-PCIe devices.

Allen

-----Original Message-----
From: Jan Beulich [mailto:JBeulich@novell.com] 
Sent: Monday, August 15, 2011 9:25 AM
To: Kay, Allen M
Cc: xen-devel@lists.xensource.com
Subject: enable_ats_device() call site

Allen,

what is the reason for calling this from VT-d's domain_context_mapping()?
I neither undertsand why this is VT-d specific, nor why it needs to be
re-done with each device re-assignment.

I'm asking because this depends on MMCFG availability, and hence the
initial call from pci_add_device() (in the context of scan_pci_devices())
to iommu_add_device() may not result in this getting enabled, while on
the first Dom0-invoked pci_add_device() pdev->domain is already set
and hence iommu_add_device() doesn't get called at all. I'd therefore
like to pull this out into pci_add_device() (and call it, together with
pci_enable_acs(), after the conditional around iommu_add_device() -
should be safe as I view both enabling functions as idempotent).

Alternatively - why do we need scan_pci_devices() at all? We're
supposed to be getting the devices reported from Dom0 anyway.

Jan

RE: enable_ats_device() call site

[Jan Beulich]

>>> On 18.08.11 at 01:27, "Kay, Allen M" <allen.m.kay@intel.com> wrote:
>>  what is the reason for calling this from VT-d's domain_context_mapping()?
>> I neither undertsand why this is VT-d specific, nor why it needs to be
>> re-done with each device re-assignment.
> 
> The reason is FLR clears the ATS enabled bit so we need to re-enable it for 
> every re-assignment.  The reason we don't need to do this for ACS might be ACS 
> reside on the bridge, not in the PCI endpoint.  ATS on the other hand, 
> resides in PCI endpoints.

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.

>> Alternatively - why do we need scan_pci_devices() at all? We're
>> supposed to be getting the devices reported from Dom0 anyway
> 
> Looks like it is use for building bus2bridge[] which is used for figuring 
> out upstream bridges which are needed when assigning non-PCIe devices.

Oh, right, I keep forgetting that, especially as that puts under question
why we have Dom0 report non-extfn, non-virtfn devices at all. And
perhaps we should issue a warning if Dom0 reports such a device that
we didn't know about already?

Jan