On 5/19/23 04:45, Jan Beulich wrote:
> On 18.05.2023 23:06, Stewart Hildebrand wrote:
>> --- a/xen/include/xen/iommu.h
>> +++ b/xen/include/xen/iommu.h
>> @@ -26,6 +26,7 @@
>> #include <xen/spinlock.h>
>> #include <public/domctl.h>
>> #include <public/hvm/ioreq.h>
>> +#include <asm/acpi.h>
>> #include <asm/device.h>
>
> I view this as problematic: It'll require all architectures with an
> IOMMU implementation to have an asm/acpi.h. I think this wants to go
> inside an "#ifdef CONFIG_ACPI" and then ...
Will do
>> @@ -228,12 +230,25 @@ int iommu_release_dt_devices(struct domain *d);
>> * (IOMMU is not enabled/present or device is not connected to it).
>> */
>> int iommu_add_dt_device(struct dt_device_node *np);
>> +int iommu_add_dt_pci_sideband_ids(struct pci_dev *pdev);
>>
>> int iommu_do_dt_domctl(struct xen_domctl *, struct domain *,
>> XEN_GUEST_HANDLE_PARAM(xen_domctl_t));
>>
>> +#else /* !HAS_DEVICE_TREE */
>> +static inline int iommu_add_dt_pci_sideband_ids(struct pci_dev *pdev)
>> +{
>> + return 0;
>> +}
>> #endif /* HAS_DEVICE_TREE */
>>
>> +static inline int iommu_add_pci_sideband_ids(struct pci_dev *pdev)
>> +{
>> + if ( acpi_disabled )
>
> ... the same #ifdef would be added around this if().
Okay. I will take care to avoid an unreachable return 0; by introducing a local
variable:
static inline int iommu_add_pci_sideband_ids(struct pci_dev *pdev)
{
int ret = 0;
#ifdef CONFIG_ACPI
if ( acpi_disabled )
#endif
ret = iommu_add_dt_pci_sideband_ids(pdev);
return ret;
}
> All of this of course only if this is deemed enough to allow co-existance
> of DT and ACPI (which I'm not convinced it is, but I don't know enough
> about DT and e.g. possible mixed configurations).
>
> Jan
On ARM, we dynamically check for the existence of a valid device tree and set
acpi_disabled accordingly. I did some basic testing on ARM with both
CONFIG_ACPI=y and # CONFIG_ACPI is not set. My understanding is that it will
work, and it should allow ACPI on ARM to be implemented in future.
>> + return iommu_add_dt_pci_sideband_ids(pdev);
>> + return 0;
>> +}
>> +
>> struct page_info;
>>
>> /*
>
