Re: [PATCH 11/17] hvmloader: allocate MMCONFIG area in the MMIO hole

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.05.2026 14:23, Roger Pau Monné wrote:
> On Mon, May 04, 2026 at 01:11:44PM +0200, Jan Beulich wrote:
>> On 29.04.2026 11:29, Roger Pau Monné wrote:
>>> On Fri, Mar 13, 2026 at 04:35:04PM +0000, Thierry Escande wrote:
>>>> --- a/tools/firmware/hvmloader/pci.c
>>>> +++ b/tools/firmware/hvmloader/pci.c
>>>> @@ -413,6 +413,58 @@ void pci_setup(void)
>>>>          pci_devfn_decode_type[devfn] |= PCI_COMMAND_MASTER;
>>>>      }
>>>>  
>>>> +    /*
>>>> +     *  Calculate MMCONFIG area size and squeeze it into the bars array
>>>> +     *  for assigning a slot in the MMIO hole
>>>> +     */
>>>> +    if ( is_running_on_q35 )
>>>> +    {
>>>> +        /* disable PCIEXBAR decoding for now */
>>>> +        pci_writel(PCI_MCH_DEVFN, PCI_MCH_PCIEXBAR, 0);
>>>> +        pci_writel(PCI_MCH_DEVFN, PCI_MCH_PCIEXBAR + 4, 0);
>>>> +
>>>> +        switch ( PCI_MAX_MCFG_BUSES )
>>>> +        {
>>>> +        case 64:
>>>> +            bar_data = PCIEXBAR_64_BUSES | PCIEXBAR_ENABLE;
>>>> +            bar_sz = MB(64);
>>>> +            break;
>>>> +
>>>> +        case 128:
>>>> +            bar_data = PCIEXBAR_128_BUSES | PCIEXBAR_ENABLE;
>>>> +            bar_sz = MB(128);
>>>> +            break;
>>>> +
>>>> +        case 256:
>>>> +            bar_data = PCIEXBAR_256_BUSES | PCIEXBAR_ENABLE;
>>>> +            bar_sz = MB(256);
>>>> +            break;
>>>> +
>>>> +        default:
>>>> +            /* unsupported number of buses specified */
>>>> +            BUG();
>>>> +        }
>>>> +
>>>> +        addr_mask = ~(bar_sz - 1);
>>>> +
>>>> +        for ( i = 0; i < nr_bars; i++ )
>>>> +            if ( bars[i].bar_sz < bar_sz )
>>>> +                break;
>>>> +
>>>> +        if ( i != nr_bars )
>>>> +            memmove(&bars[i+1], &bars[i], (nr_bars-i) * sizeof(*bars));
>>>> +
>>>> +        bars[i].is_mem    = 1;
>>>> +        bars[i].devfn     = PCI_MCH_DEVFN;
>>>> +        bars[i].bar_reg   = PCI_MCH_PCIEXBAR;
>>>> +        bars[i].bar_sz    = bar_sz;
>>>> +        bars[i].addr_mask = addr_mask;
>>>> +        bars[i].bar_data  = bar_data;
>>>> +
>>>> +        mmio_total += bar_sz;
>>>> +        nr_bars++;
>>>> +    }
>>>
>>> I think it might be best if the ECAM fake BAR is the first element in
>>> the bars array, so we ensure it's the first item to consume memory
>>> from the low MMIO hole.  Not sure how that will work with the current
>>> sorting of the resources based on their size, but it's imperative for
>>> hvmloader to attempt to position ECAM ahead of the other device
>>> resources IMO.
>>
>> Why would this be?
> 
> I would assume it's best to have ECAM access in the low 4G (for 32bit
> OSes) at the expense of some 32bit BARs possibly not fitting in the
> 32bit space.  But the ECAM space could be placed above 4G, and 32bit
> OSes might not care much about extended address space capabilities.

This doesn't require it to be 1st, though? Ideally we would try to put
it below 4G if possible (i.e. if all other BARs which aren't 64-bit
capable still fit), but fall back to putting it above 4G otherwise. In
all of this it would still be put in the slot that its size demands.
If we put it first, a significant gap could result between it and the
first "real" BAR. (Then again a significant gap could also result if
the number of buses to cover wasn't a power of 2.) Properly making use
of such a gap would further complicate the code, I expect.

> Should is_64bar be set for the MCFG "fake" BAR?

As per above, maybe when making a 2nd (retry) pass.

Jan