Messages in this thread | | | Subject | Re: [PATCH 2/2] xen/pv: Call get_cpu_address_sizes to set x86_virt/phys_bits | From | "M. Vefa Bicakci" <> | Date | Tue, 24 Jul 2018 08:42:28 -0400 |
| |
On 07/23/2018 11:04 AM, Boris Ostrovsky wrote: > On 07/22/2018 11:57 AM, M. Vefa Bicakci wrote: >> On 07/21/2018 07:17 PM, M. Vefa Bicakci wrote: >>> On 07/21/2018 05:25 PM, Boris Ostrovsky wrote: >>>> On 07/21/2018 03:49 PM, M. Vefa Bicakci wrote: >>>>> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c >>>>> index 439a94bf89ad..87afb000142a 100644 >>>>> --- a/arch/x86/xen/enlighten_pv.c >>>>> +++ b/arch/x86/xen/enlighten_pv.c >>>>> @@ -1257,6 +1257,7 @@ asmlinkage __visible void __init >>>>> xen_start_kernel(void) >>>>> /* Work out if we support NX */ >>>>> get_cpu_cap(&boot_cpu_data); >>>>> + get_cpu_address_sizes(&boot_cpu_data); >>>>> x86_configure_nx(); >>>> >>>> >>>> Have you observed any problems without this call? get_cpu_cap() is only >>>> called here to set X86_FEATURE_NX, and is then called again, together >>>> with get_cpu_address_sizes(), from early_identify_cpu(). >>> >>> Thank you for the reviews! Without the call to get_cpu_address_sizes, >>> paravirtualized virtual machines do not boot up kernels with versions >>> 4.17 and up at all; this includes dom0 and domU. No domU logs are >>> generated in dom0's /var/log/xen/console/ directory either, despite >>> having earlyprintk=xen on the kernel command line for my test domU. >> >> Hello Boris, >> >> I debugged this further with a debugging version of Xen (so that I can >> get early kernel print-outs via the "xen_raw_console_write" function), >> and I found the root cause of the boot up failure. >> >> In summary, the issue is due to the following call path in version >> 4.17 (and higher, I assume), which the kernel goes through /only/ when >> CONFIG_DEBUG_VIRTUAL is enabled: >> >> enlighten_pv.c::xen_start_kernel >> mmu_pv.c::xen_reserve_special_pages >> page.h::__pa >> physaddr.c::__phys_addr >> physaddr.h::phys_addr_valid // uses boot_cpu_data.x86_phys_bits >> >> The return value of phys_addr_valid is used with the VIRTUAL_BUG_ON >> macro, >> which evaluates to BUG_ON in case CONFIG_DEBUG_VIRTUAL is enabled. > > > Ah, that's why it hasn't been detected. > > >> >> It looks like the call to get_cpu_address_size is required in the >> xen_start_kernel function. Perhaps there is a more elegant way to >> resolve this issue as well. >> >> Another approach could be to check in the phys_addr_valid function >> whether >> boot_cpu_data.x86_phys_bits has been initialized or not, I think, but >> I am >> not sure about the correctness of this approach. > > > No, I think your patch is good. The only thing I'd suggest is to move > the call a few lines down. The way it is placed now may create > impression that we are calling get_cpu_address_sizes() to figure out NX > support.
Sorry for the delay, and thank you for your comments! I will send an updated version of this patch in a few moments.
Vefa
| |