>>> On 19.09.16 at 17:38, <daniel.ki...@oracle.com> wrote:
> On Mon, Sep 19, 2016 at 08:57:02AM -0600, Jan Beulich wrote:
>> >>> On 19.09.16 at 16:27, <daniel.ki...@oracle.com> wrote:
>> > On Mon, Sep 19, 2016 at 05:58:46AM -0600, Jan Beulich wrote:
>> >> >>> On 12.09.16 at 22:18, <daniel.ki...@oracle.com> wrote:
>> >> > --- a/xen/arch/x86/domain_page.c
>> >> > +++ b/xen/arch/x86/domain_page.c
>> >> > @@ -36,7 +36,7 @@ static inline struct vcpu *mapcache_current_vcpu(void)
>> >> > * domain's page tables but current may point at another domain's
>> > VCPU.
>> >> > * Return NULL as though current is not properly set up yet.
>> >> > */
>> >> > - if ( efi_enabled && efi_rs_using_pgtables() )
>> >> > + if ( efi_enabled(EFI_RS) && efi_rs_using_pgtables() )
>> >> I think the efi_enabled() here is pointless now.
>> > Nope, it seems that Xen will blow up on BUG() in
>> > xen/arch/x86/efi/stub.c:efi_rs_using_pgtables() if
>> > compiler/linker cannot be used to build proper PE binary.
>> Ah, true.
>> > Of course we can change efi_rs_using_pgtables() to
>> > return false in such case.
>> Except that it does already. You mean dropping the BUG() I guess.
> Yep, right. However, should not we change "return 0" to "return false"
> in the same patch if efi_rs_using_pgtables() returns bool_t?
Since you don't touch that line anyway, and since using false
there would then also call for switching from bool_t to bool, I'd
rather leave this for another day.
>> Both ways should be fine then for now.
> Dropping the BUG() from efi_rs_using_pgtables() and efi_enabled in
> above mentioned conditional makes final code, IMO, better. However,
> in this patch context it may look a bit strange. Is it acceptable
> for you to do both in this patch?
Yes, that what I tried to express with my previous reply.
Xen-devel mailing list