On 09/12/2025 4:42 pm, Jan Beulich wrote:
> On 09.12.2025 17:06, Grygorii Strashko wrote:
>> On 09.12.25 17:57, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -891,7 +891,7 @@ int arch_domain_create(struct domain *d,
>>>       d->arch.emulation_flags = emflags;
>>>   
>>>   #ifdef CONFIG_PV32
>>> -    HYPERVISOR_COMPAT_VIRT_START(d) =
>>> +    d->arch.hv_compat_vstart =
>>>           is_pv_domain(d) ? __HYPERVISOR_COMPAT_VIRT_START : ~0u;
>>>   #endif
>> Any chances it can be moved in pv_domain_initialise()?
> Probably, but one thing at a time? The field itself would also want to move
> from struct arch_domain to struct pv_domain, I think.

Agreed to one thing at a time.

The value itself is a total mess.  Storage exists based on CONFIG_PV32,
with 0 yielded in !PV32 builds.

Yet, in CONFIG_PV32 builds, it has the value 0xF5800000 for PV domains,
and 0xFFFFFFFF for HVM domains. 

This is nonsense, causing e.g. COMPAT_L2_PAGETABLE_FIRST_XEN_SLOT() to
return answers at opposite ends of the pagetable for non-PV32 VMs
depending on CONFIG_PV32.  I think this all works because the logic is
behind suitable is_pv32_$FOO() checks, but it's far from clear.

It is only a PV32 dom0 which can have this set to anything besides
0xF5800000, so the "correct" thing to do would be to leave it 0 in
domain create, and set it to 0xF5800000 in switch_compat(), along with
the custom setup in dom0_construct().

But, lets get the d->arch.physaddr_bitsize adjustments sorted first
before conflicting those with this change.

~Andrew

Reply via email to