Re: [PATCH 1/3] x86: tolerate running on EFI runtime services page tables in map_domain_page()

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 23/10/14 16:18, Jan Beulich wrote:
>>>> On 23.10.14 at 15:57, <andrew.cooper3@citrix.com> wrote:
>> On 23/10/14 14:44, Jan Beulich wrote:
>>> --- a/xen/arch/x86/domain_page.c
>>> +++ b/xen/arch/x86/domain_page.c
>>> @@ -7,6 +7,7 @@
>>>   */
>>>  
>>>  #include <xen/domain_page.h>
>>> +#include <xen/efi.h>
>>>  #include <xen/mm.h>
>>>  #include <xen/perfc.h>
>>>  #include <xen/pfn.h>
>>> @@ -37,11 +38,14 @@ static inline struct vcpu *mapcache_curr
>>>       */
>>>      if ( unlikely(pagetable_is_null(v->arch.guest_table)) && is_pv_vcpu(v) )
>>>      {
>>> +        unsigned long cr3;
>>> +
>> This will need an __maybe_unused to compile in a non-debug build.
> Definitely not (and I actually build tested it also for that case).
> ASSERT() had got changed a (long) while ago to specifically allow
> for such cases.
>
>>> --- a/xen/arch/x86/efi/stub.c
>>> +++ b/xen/arch/x86/efi/stub.c
>>> @@ -9,6 +9,12 @@ const bool_t efi_enabled = 0;
>>>  
>>>  void __init efi_init_memory(void) { }
>>>  
>>> +paddr_t efi_rs_page_table(void)
>>> +{
>>> +    BUG();
>>> +    return 0;
>> Is the return strictly needed?  The __builtin_unreachable() in BUG()
>> should prevent the compiler from complaining.
> It's not strictly needed, but is in line with the immediately following
> function.
>
> Jan
>

Ok.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>