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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

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

Thanks, Roger.