Re: [PATCH v6 2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Stewart,

> On 29 Nov 2023, at 9:25 pm, Stewart Hildebrand <Stewart.Hildebrand@xxxxxxx> 
> wrote:
> 
> On 11/14/23 04:11, Jan Beulich wrote:
>> On 13.11.2023 23:21, Stewart Hildebrand wrote:
>>> @@ -709,10 +710,17 @@ int arch_sanitise_domain_config(struct 
>>> xen_domctl_createdomain *config)
>>>         return -EINVAL;
>>>     }
>>> 
>>> +    if ( vpci && !hvm )
>>> +    {
>>> +        dprintk(XENLOG_INFO, "vPCI requested for non-HVM guest\n");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>>     return 0;
>>> }
>> 
>> As said on the v5 thread, I think my comment was misguided (I'm sorry)
>> and this wants keeping in common code as you had it.
> 
> I'll move it back to xen/common/domain.c. No worries.

I tested this patch and observed build failure when compiling the "x86_64” arch 
with
"CONFIG_HVM=n“ option.

x86_64-linux-gnu-ld    -melf_x86_64  -T arch/x86/xen.lds -N prelink.o 
--build-id=sha1 \
    ./common/symbols-dummy.o -o ./.xen-syms.0 
x86_64-linux-gnu-ld: prelink.o: in function `arch_iommu_hwdom_init’:
(.init.text+0x2192b): undefined reference to `vpci_is_mmcfg_address’
(.init.text+0x2192b): relocation truncated to fit: R_X86_64_PLT32 against 
undefined symbol `vpci_is_mmcfg_address'
x86_64-linux-gnu-ld: (.init.text+0x21947): undefined reference to 
`vpci_is_mmcfg_address'
(.init.text+0x21947): relocation truncated to fit: R_X86_64_PLT32 against 
undefined symbol `vpci_is_mmcfg_address'
x86_64-linux-gnu-ld: prelink.o: in function `do_physdev_op’: 
(.text.do_physdev_op+0x6db): undefined reference to 
`register_vpci_mmcfg_handler'
(.text.do_physdev_op+0x6db): relocation truncated to fit: R_X86_64_PLT32 
against undefined symbol `register_vpci_mmcfg_handler'
x86_64-linux-gnu-ld: ./.xen-syms.0: hidden symbol `vpci_is_mmcfg_address' isn't 
defined
x86_64-linux-gnu-ld: final link failed: bad value                


Regards,
Rahul


> 
>> 
>>> --- a/xen/include/public/arch-x86/xen.h
>>> +++ b/xen/include/public/arch-x86/xen.h
>>> @@ -283,15 +283,16 @@ struct xen_arch_domainconfig {
>>> #define XEN_X86_EMU_PIT             (1U<<_XEN_X86_EMU_PIT)
>>> #define _XEN_X86_EMU_USE_PIRQ       9
>>> #define XEN_X86_EMU_USE_PIRQ        (1U<<_XEN_X86_EMU_USE_PIRQ)
>>> -#define _XEN_X86_EMU_VPCI           10
>>> -#define XEN_X86_EMU_VPCI            (1U<<_XEN_X86_EMU_VPCI)
>>> +/*
>>> + * Note: bit 10 was previously used for a XEN_X86_EMU_VPCI flag. This bit 
>>> should
>>> + * not be re-used without careful consideration.
>>> + */
>> 
>> I think a multi-line comment is drawing overly much attention to this.
>> How about "Note: Bit 10 was previously used for XEN_X86_EMU_VPCI. Re-use
>> with care." which I think fits in a single line comment.
> 
> Sounds good.
> 
>> 
>> Jan