Re: [PATCH v5 4/5] i386/pc: relocate 4g start to 1T where applicable

[Igor Mammedov]

On Fri, 17 Jun 2022 14:33:02 +0100
Joao Martins <joao.m.martins@oracle.com> wrote:

> On 6/17/22 13:32, Igor Mammedov wrote:
> > On Fri, 17 Jun 2022 13:18:38 +0100
> > Joao Martins <joao.m.martins@oracle.com> wrote:  
> >> On 6/16/22 15:23, Igor Mammedov wrote:  
> >>> On Fri, 20 May 2022 11:45:31 +0100
> >>> Joao Martins <joao.m.martins@oracle.com> wrote:  
> >>>> +                                hwaddr above_4g_mem_start,
> >>>> +                                uint64_t pci_hole64_size)
> >>>> +{
> >>>> +    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> >>>> +    X86MachineState *x86ms = X86_MACHINE(pcms);
> >>>> +    MachineState *machine = MACHINE(pcms);
> >>>> +    ram_addr_t device_mem_size = 0;
> >>>> +    hwaddr base;
> >>>> +
> >>>> +    if (!x86ms->above_4g_mem_size) {
> >>>> +       /*
> >>>> +        * 32-bit pci hole goes from
> >>>> +        * end-of-low-ram (@below_4g_mem_size) to IOAPIC.
> >>>> +        */
> >>>> +        return IO_APIC_DEFAULT_ADDRESS - 1;    
> >>>
> >>> lack of above_4g_mem, doesn't mean absence of device_mem_size or anything 
> >>> else
> >>> that's located above it.
> >>>     
> >>
> >> True. But the intent is to fix 32-bit boundaries as one of the qtests was 
> >> failing
> >> otherwise. We won't hit the 1T hole, hence a nop.  
> > 
> > I don't get the reasoning, can you clarify it pls?
> >   
> 
> I was trying to say that what lead me here was a couple of qtests failures 
> (from v3->v4).
> 
> I was doing this before based on pci_hole64. phys-bits=32 was for example one
> of the test failures, and pci-hole64 sits above what 32-bit can reference.

if user sets phys-bits=32, then nothing above 4Gb should work (be usable)
(including above-4g-ram, hotplug region or pci64 hole or sgx or cxl)

and this doesn't look to me as AMD specific issue

perhaps do a phys-bits check as a separate patch
that will error out if max_used_gpa is above phys-bits limit
(maybe at machine_done time)
(i.e. defining max_gpa and checking if compatible with configured cpu
are 2 different things)

(it might be possible that tests need to be fixed too to account for it)

> >>  Unless we plan on using
> >> pc_max_used_gpa() for something else other than this.  
> > 
> > Even if '!above_4g_mem_sizem', we can still have hotpluggable memory region
> > present and that can  hit 1Tb. The same goes for pci64_hole if it's 
> > configured
> > large enough on CLI.
> >   
> So hotpluggable memory seems to assume it sits above 4g mem.
> 
> pci_hole64 likewise as it uses similar computations as hotplug.
> 
> Unless I am misunderstanding something here.
> 
> > Looks like guesstimate we could use is taking pci64_hole_end as max used GPA
> >   
> I think this was what I had before (v3[0]) and did not work.

that had been tied to host's phys-bits directly, all in one patch
and duplicating existing pc_pci_hole64_start().
 
> Let me revisit this edge case again.
> 
> [0] 
> https://lore.kernel.org/all/20220223184455.9057-5-joao.m.martins@oracle.com/
>