On 2025/7/8 22:10, Jan Beulich wrote:
> On 04.07.2025 09:07, Jiqian Chen wrote:
>> --- a/xen/drivers/vpci/header.c
>> +++ b/xen/drivers/vpci/header.c
>> @@ -836,6 +836,39 @@ static int vpci_init_capability_list(struct pci_dev
>> *pdev)
>> PCI_STATUS_RSVDZ_MASK);
>> }
>>
>> +static int vpci_init_ext_capability_list(const struct pci_dev *pdev)
>> +{
>> + unsigned int pos = PCI_CFG_SPACE_SIZE;
>> +
>> + if ( !is_hardware_domain(pdev->domain) )
>> + /* Extended capabilities read as zero, write ignore for DomU */
>> + return vpci_add_register(pdev->vpci, vpci_read_val, NULL,
>> + pos, 4, (void *)0);
>> +
>> + do
>> + {
>> + uint32_t header = pci_conf_read32(pdev->sbdf, pos);
>> + int rc;
>> +
>> + rc = vpci_add_register(pdev->vpci, vpci_read_val, vpci_hw_write32,
>> + pos, 4, (void *)(uintptr_t)header);
>
> If it wasn't for this use of vpci_hw_write32(), I'd be happy to provide my
> R-b. But this continues to look bogus to me: What use is it to allow writes
> when Dom0 then can't read back any possible effect of such a write (in the
> unexpected event that some of the bits were indeed writable)?
Oh, I got your concern.
What do you think about updating the header value after writing to hardware in
write function?
Or you prefer to pass NULL here?
>
> Jan
--
Best regards,
Jiqian Chen.
