Re: [PATCH v2 1/1] vpci: Add resizable bar support

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.12.2024 12:00, Roger Pau Monné wrote:
> On Mon, Dec 09, 2024 at 02:59:31PM +0100, Jan Beulich wrote:
>> On 02.12.2024 07:09, Jiqian Chen wrote:
>>> +static int cf_check init_rebar(struct pci_dev *pdev)
>>> +{
>>> +    uint32_t ctrl;
>>> +    unsigned int rebar_offset, nbars;
>>> +
>>> +    rebar_offset = pci_find_ext_capability(pdev->sbdf, 
>>> PCI_EXT_CAP_ID_REBAR);
>>> +
>>> +    if ( !rebar_offset )
>>> +        return 0;
>>> +
>>> +    ctrl = pci_conf_read32(pdev->sbdf, rebar_offset + PCI_REBAR_CTRL);
>>> +    nbars = MASK_EXTR(ctrl, PCI_REBAR_CTRL_NBAR_MASK);
>>> +
>>> +    for ( unsigned int i = 0; i < nbars; i++, rebar_offset += 
>>> PCI_REBAR_CTRL )
>>> +    {
>>> +        int rc;
>>> +
>>> +        rc = vpci_add_register(pdev->vpci, vpci_hw_read32, vpci_hw_write32,
>>> +                               rebar_offset + PCI_REBAR_CAP, 4, NULL);
>>
>> The capability register is r/o aiui. While permitting hwdom to write it is
>> fine, DomU-s shouldn't be permitted doing so, just in case. (An alternative
>> to making handler selection conditional here would be to bail early for the
>> !hwdom case, accompanied by a TODO comment. This would then also address
>> the lack of virtualization of the extended capability chain, as we may not
>> blindly expose all capabilities to DomU-s.)
> 
> I don't think we can safely expose this capability to domUs by
> default, so my preference would be a returning an error in that case
> (and printing a log message indicating ReBAR is not supported for
> domUs).

I understood Jiqian's recent reply to mean that that's what he's going to do.

> Note it's already not exposed to domUs by not being part of
> supported_caps in init_header().

Just to mention it - supported_caps is, aiui, about "traditional" caps only
anyway, not extended ones.

Jan