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
