Re: [PATCH v3 10/17] xen/arm: Add cmdline boot option "pci-passthrough = <boolean>"

[Rahul Singh <Rahul.Singh@arm.com>]

Hi Jan,

> On 30 Sep 2021, at 4:00 pm, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 28.09.2021 20:18, Rahul Singh wrote:
>> @@ -62,8 +63,21 @@ static int __init acpi_pci_init(void)
>> }
>> #endif
>> 
>> +/*
>> + * By default pci passthrough is disabled
>> + */
> 
> Nit: This is a single line comment.
Ack.
> 
>> +bool __read_mostly pci_passthrough_enabled = false;
> 
> Nit: Unnecessary initializer.

Ack. 
> 
>> --- a/xen/common/physdev.c
>> +++ b/xen/common/physdev.c
>> @@ -19,6 +19,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>         struct pci_dev_info pdev_info;
>>         nodeid_t node;
>> 
>> +        if ( !is_pci_passthrough_enabled() )
>> +            return -ENOSYS;
> 
> Here and ...
> 
>> @@ -54,6 +57,9 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>     case PHYSDEVOP_pci_device_remove: {
>>         struct physdev_pci_device dev;
>> 
>> +        if ( !is_pci_passthrough_enabled() )
>> +            return -ENOSYS;
> 
> ... here -EOPNOTSUPP (or any other suitable value, which -ENOSYS is not
> from all I can tell) please.

I checked all the possible errors and came to the conclusion that
 -EOPNOTSUPP is the right choice here.

> 
>> --- a/xen/include/asm-x86/pci.h
>> +++ b/xen/include/asm-x86/pci.h
>> @@ -32,4 +32,12 @@ bool_t pci_ro_mmcfg_decode(unsigned long mfn, unsigned int *seg,
>> extern int pci_mmcfg_config_num;
>> extern struct acpi_mcfg_allocation *pci_mmcfg_config;
>> 
>> +/*
>> + * Unlike ARM, PCI passthrough is always enabled for x86.
>> + */
> 
> Nit: This again is a single line comment.

Ack.

Regards,
Rahul
> 
> Jan
>