On 06.08.2025 05:39, Chen, Jiqian wrote:
> On 2025/8/5 16:48, Jan Beulich wrote:
>> On 05.08.2025 05:49, Jiqian Chen wrote:
>>> --- a/xen/drivers/vpci/rebar.c
>>> +++ b/xen/drivers/vpci/rebar.c
>>> @@ -49,6 +49,32 @@ static void cf_check rebar_ctrl_write(const struct
>>> pci_dev *pdev,
>>> bar->guest_addr = bar->addr;
>>> }
>>>
>>> +static int cf_check cleanup_rebar(const struct pci_dev *pdev)
>>> +{
>>> + int rc;
>>> + uint32_t ctrl;
>>> + unsigned int nbars;
>>> + unsigned int rebar_offset = pci_find_ext_capability(pdev->sbdf,
>>> +
>>> PCI_EXT_CAP_ID_REBAR);
>>> +
>>> + if ( !rebar_offset || !is_hardware_domain(pdev->domain) )
>>> + {
>>> + ASSERT_UNREACHABLE();
>>> + return 0;
>>> + }
>>> +
>>> + ctrl = pci_conf_read32(pdev->sbdf, rebar_offset + PCI_REBAR_CTRL(0));
>>> + nbars = MASK_EXTR(ctrl, PCI_REBAR_CTRL_NBAR_MASK);
>>> +
>>> + rc = vpci_remove_registers(pdev->vpci, rebar_offset + PCI_REBAR_CAP(0),
>>> + PCI_REBAR_CTRL(nbars - 1));
>>> + if ( rc )
>>> + printk(XENLOG_ERR "%pd %pp: fail to remove Rebar handlers rc=%d\n",
>>> + pdev->domain, &pdev->sbdf, rc);
>>
>> MSI and MSI-X (now) have ASSERT_UNREACHABLE() on their respective paths. Is
>> there a reason this shouldn't be done here as well?
> Will add.
>
>>
>> MSI and MSI-X further have another add-register below here, to ensure the
>> control register cannot be written. Again - is there a reason the same
>> shouldn't be done here? (If so, I think this may want putting in a comment.)
> Since extended capabilities are only exposed to dom0, and dom0 has no
> limitations to access devices.
> It since there is not much point in adding such a handler for rebar.
While the effect is different from MSI / MSI-X, isn't it still a problem if
Dom0 changed BAR sizes entirely behind Xen's back? Which could be prevented
by similarly discarding writes to the control register, as is done for the
other two?
Jan
