Re: Runtime services support for Xen on ARM

Re: Runtime services support for Xen on ARM

[Stefano Stabellini]

CC'ing xen-devel and Jan

On Tue, 10 Nov 2015, Shannon Zhao wrote:
> Hi Stefano,
> 
> I'm working on adding Runtime services support at Xen side. Most of work
> is adding the ARM part in xen/common/efi/runtime.c.
> 
> There is one problem which block me. That is how to implement
> efi_rs_enter() and efi_rs_leave() for ARM, since I think current
> implementation is x86 specific and won't work on ARM. Also the rtc_lock.
> 
> Could you give some suggestion? Thanks!

efi_rs_enter() and efi_rs_leave() look very PV x86 specific. It is
possible that we don't actually need to do anything at all on ARM, aside
from refactoring the code. Jan?

Similarly we don't have an rtc_lock spinlock today and it is not
actually needed, but for the sake of keeping the common code common, I
would just introduce rtc_lock on ARM too.

Re: Runtime services support for Xen on ARM

[Ian Campbell]

On Tue, 2015-11-10 at 12:26 +0000, Stefano Stabellini wrote:
> CC'ing xen-devel and Jan
> 
> On Tue, 10 Nov 2015, Shannon Zhao wrote:
> > Hi Stefano,
> > 
> > I'm working on adding Runtime services support at Xen side. Most of
> > work
> > is adding the ARM part in xen/common/efi/runtime.c.
> > 
> > There is one problem which block me. That is how to implement
> > efi_rs_enter() and efi_rs_leave() for ARM, since I think current
> > implementation is x86 specific and won't work on ARM. Also the
> > rtc_lock.
> > 
> > Could you give some suggestion? Thanks!
> 
> efi_rs_enter() and efi_rs_leave() look very PV x86 specific. It is
> possible that we don't actually need to do anything at all on ARM, aside
> from refactoring the code. Jan?

I think these functions derive from the USE_SET_VIRTUAL_ADDRESS_MAP option.
If that is set we call efi_rs->SetVirtualAddressMap to tell the f/w about
our virtual address layout and can then call rs directly with no faff.

Unfortunately SetVirtualAddressMap is incompatible with kexec (I think you
can only call it once), so we have this USE_SET_VIRTUAL_ADDRESS_MAP option.

Without USE_SET_VIRTUAL_ADDRESS_MAP we need to switch to a set of page
tables which are "OK" for calling the runtime services. I'm not sure what
"OK" means here -- but I suspect it means "1:1" in some part.

So sadly I think we do _eventually_ need to support this mode (for kexec),
which would mean quite a bit of refactoring (since the relevant code
in xen/common/efi/boot.c has some x86-isms).

But right now, since we do not yet support kexec,  I think we could get the
ball rolling wrt RS support by setting USE_SET_VIRTUAL_ADDRESS_MAP on ARM
and dodging the need to make efi_rs_enter() work right now. (IOW make it
the problem of whomever wants kexec support...)

Unless there are issues other than kexec with SetVirtualAddressMap which
I'm not thinking of now?

> Similarly we don't have an rtc_lock spinlock today and it is not
> actually needed, but for the sake of keeping the common code common, I
> would just introduce rtc_lock on ARM too.

Agreed.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: Runtime services support for Xen on ARM

[Shannon Zhao]

On 2015/11/10 20:36, Ian Campbell wrote:
> On Tue, 2015-11-10 at 12:26 +0000, Stefano Stabellini wrote:
>> CC'ing xen-devel and Jan
>>
>> On Tue, 10 Nov 2015, Shannon Zhao wrote:
>>> Hi Stefano,
>>>
>>> I'm working on adding Runtime services support at Xen side. Most of
>>> work
>>> is adding the ARM part in xen/common/efi/runtime.c.
>>>
>>> There is one problem which block me. That is how to implement
>>> efi_rs_enter() and efi_rs_leave() for ARM, since I think current
>>> implementation is x86 specific and won't work on ARM. Also the
>>> rtc_lock.
>>>
>>> Could you give some suggestion? Thanks!
>>
>> efi_rs_enter() and efi_rs_leave() look very PV x86 specific. It is
>> possible that we don't actually need to do anything at all on ARM, aside
>> from refactoring the code. Jan?
>
> I think these functions derive from the USE_SET_VIRTUAL_ADDRESS_MAP option.
> If that is set we call efi_rs->SetVirtualAddressMap to tell the f/w about
> our virtual address layout and can then call rs directly with no faff.
>
> Unfortunately SetVirtualAddressMap is incompatible with kexec (I think you
> can only call it once), so we have this USE_SET_VIRTUAL_ADDRESS_MAP option.
>
> Without USE_SET_VIRTUAL_ADDRESS_MAP we need to switch to a set of page
> tables which are "OK" for calling the runtime services. I'm not sure what
> "OK" means here -- but I suspect it means "1:1" in some part.
>
> So sadly I think we do _eventually_ need to support this mode (for kexec),
> which would mean quite a bit of refactoring (since the relevant code
> in xen/common/efi/boot.c has some x86-isms).
>
> But right now, since we do not yet support kexec,  I think we could get the
> ball rolling wrt RS support by setting USE_SET_VIRTUAL_ADDRESS_MAP on ARM
> and dodging the need to make efi_rs_enter() work right now. (IOW make it
> the problem of whomever wants kexec support...)
>
Yeah, we could set USE_SET_VIRTUAL_ADDRESS_MAP at this moment, but as 
you said, it needs to support kexec and make efi_rs_enter() work 
eventually. So if anyone know this well, please share me your ideas. 
Thanks very much.

> Unless there are issues other than kexec with SetVirtualAddressMap which
> I'm not thinking of now?
>
>> Similarly we don't have an rtc_lock spinlock today and it is not
>> actually needed, but for the sake of keeping the common code common, I
>> would just introduce rtc_lock on ARM too.
>
> Agreed.
>

-- 
Shannon

Re: Runtime services support for Xen on ARM

[Shannon Zhao]

On 2015/11/10 20:36, Ian Campbell wrote:
> On Tue, 2015-11-10 at 12:26 +0000, Stefano Stabellini wrote:
>> CC'ing xen-devel and Jan
>>
>> On Tue, 10 Nov 2015, Shannon Zhao wrote:
>>> Hi Stefano,
>>>
>>> I'm working on adding Runtime services support at Xen side. Most of
>>> work
>>> is adding the ARM part in xen/common/efi/runtime.c.
>>>
>>> There is one problem which block me. That is how to implement
>>> efi_rs_enter() and efi_rs_leave() for ARM, since I think current
>>> implementation is x86 specific and won't work on ARM. Also the
>>> rtc_lock.
>>>
>>> Could you give some suggestion? Thanks!
>>
>> efi_rs_enter() and efi_rs_leave() look very PV x86 specific. It is
>> possible that we don't actually need to do anything at all on ARM, aside
>> from refactoring the code. Jan?
> 
> I think these functions derive from the USE_SET_VIRTUAL_ADDRESS_MAP option.
> If that is set we call efi_rs->SetVirtualAddressMap to tell the f/w about
> our virtual address layout and can then call rs directly with no faff.
> 
> Unfortunately SetVirtualAddressMap is incompatible with kexec (I think you
> can only call it once), so we have this USE_SET_VIRTUAL_ADDRESS_MAP option.
> 
> Without USE_SET_VIRTUAL_ADDRESS_MAP we need to switch to a set of page
> tables which are "OK" for calling the runtime services. I'm not sure what
> "OK" means here -- but I suspect it means "1:1" in some part.
> 
> So sadly I think we do _eventually_ need to support this mode (for kexec),
> which would mean quite a bit of refactoring (since the relevant code
> in xen/common/efi/boot.c has some x86-isms).
> 
> But right now, since we do not yet support kexec,  I think we could get the
> ball rolling wrt RS support by setting USE_SET_VIRTUAL_ADDRESS_MAP on ARM
> and dodging the need to make efi_rs_enter() work right now. (IOW make it
> the problem of whomever wants kexec support...)
> 
Hi Ian,

Today I try the way you suggested. Set USE_SET_VIRTUAL_ADDRESS_MAP on
ARM and make a fake efi_rs_enter() and efi_rs_leave(). But when calling
efi_init_memory, it fails with below log:

(XEN) Hypervisor Trap. HSR=0x96000005 EC=0x25 IL=1 Syndrome=0x5
(XEN) CPU0: Unexpected Trap: Hypervisor
(XEN) ----[ Xen-4.7-unstable  arm64  debug=y  Not tainted ]----
(XEN) CPU:    0
(XEN) PC:     0000000000294564 efi_init_memory+0x4c/0x88
(XEN) LR:     000000000029455c
(XEN) SP:     00000000002bfe00
(XEN) CPSR:   800003c9 MODE:64-bit EL2h (Hypervisor, handler)

(XEN) Xen call trace:
(XEN)    [<0000000000294564>] efi_init_memory+0x4c/0x88 (PC)
(XEN)    [<000000000029455c>] efi_init_memory+0x44/0x88 (LR)
(XEN)    [<000000000028d7c0>] arch_init_memory+0x84/0x8c
(XEN)    [<000000000028f6e4>] start_xen+0xa64/0xca8
(XEN)    [<000000000020060c>] paging+0x84/0xbc

It fails at below line:

    efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
                                 mdesc_ver, efi_memmap);

While in efi_init_memory of xen/common/efi/boot.c, I see below words:

        else
        {
#ifdef USE_SET_VIRTUAL_ADDRESS_MAP
            /* XXX allocate e.g. down from FIXADDR_START */
#endif
            printk(XENLOG_ERR "No mapping for MFNs %#lx-%#lx\n",
                   smfn, emfn - 1);
        }

I don't understand this function well and what do we need to do here for
ARM?

Thanks,
-- 
Shannon

Re: Runtime services support for Xen on ARM

[Jan Beulich]

>>> On 12.11.15 at 10:06, <zhaoshenglong@huawei.com> wrote:
> Today I try the way you suggested. Set USE_SET_VIRTUAL_ADDRESS_MAP on
> ARM and make a fake efi_rs_enter() and efi_rs_leave(). But when calling
> efi_init_memory, it fails with below log:

You can't just #define that constant and assume things will work: This
code is there so far only for documentation purposes. You need to go
look what parts of it may have bitrotted.

> It fails at below line:
> 
>     efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
>                                  mdesc_ver, efi_memmap);

Quite obviously because efi_exit_boot() now sets efi_bs to NULL.

Jan

Re: Runtime services support for Xen on ARM

[Shannon Zhao]

On 2015/11/12 19:04, Jan Beulich wrote:
>>>> On 12.11.15 at 10:06, <zhaoshenglong@huawei.com> wrote:
>> Today I try the way you suggested. Set USE_SET_VIRTUAL_ADDRESS_MAP on
>> ARM and make a fake efi_rs_enter() and efi_rs_leave(). But when calling
>> efi_init_memory, it fails with below log:
>
> You can't just #define that constant and assume things will work: This
> code is there so far only for documentation purposes. You need to go
> look what parts of it may have bitrotted.
>
>> It fails at below line:
>>
>>      efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
>>                                   mdesc_ver, efi_memmap);
>
> Quite obviously because efi_exit_boot() now sets efi_bs to NULL.
>
I see in efi_start() there is below code setting efi_bs to NULL. But 
here it calls efi_rs not efi_bs. I don't understand the relation of them.

        status = SystemTable->BootServices->ExitBootServices(ImageHandle,
                                                              map_key);
         efi_bs = NULL;


-- 
Shannon

Re: Runtime services support for Xen on ARM

[Jan Beulich]

>>> On 12.11.15 at 13:52, <shannon.zhao@linaro.org> wrote:
> On 2015/11/12 19:04, Jan Beulich wrote:
>>>>> On 12.11.15 at 10:06, <zhaoshenglong@huawei.com> wrote:
>>> Today I try the way you suggested. Set USE_SET_VIRTUAL_ADDRESS_MAP on
>>> ARM and make a fake efi_rs_enter() and efi_rs_leave(). But when calling
>>> efi_init_memory, it fails with below log:
>>
>> You can't just #define that constant and assume things will work: This
>> code is there so far only for documentation purposes. You need to go
>> look what parts of it may have bitrotted.
>>
>>> It fails at below line:
>>>
>>>      efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
>>>                                   mdesc_ver, efi_memmap);
>>
>> Quite obviously because efi_exit_boot() now sets efi_bs to NULL.
>>
> I see in efi_start() there is below code setting efi_bs to NULL. But 
> here it calls efi_rs not efi_bs. I don't understand the relation of them.

Oh, sorry, my mistake. I guess I'd better not comment on ARM
crashes...

Jan

Re: Runtime services support for Xen on ARM

[Jan Beulich]

>>> On 10.11.15 at 13:26, <stefano.stabellini@eu.citrix.com> wrote:
> CC'ing xen-devel and Jan
> 
> On Tue, 10 Nov 2015, Shannon Zhao wrote:
>> Hi Stefano,
>> 
>> I'm working on adding Runtime services support at Xen side. Most of work
>> is adding the ARM part in xen/common/efi/runtime.c.
>> 
>> There is one problem which block me. That is how to implement
>> efi_rs_enter() and efi_rs_leave() for ARM, since I think current
>> implementation is x86 specific and won't work on ARM. Also the rtc_lock.
>> 
>> Could you give some suggestion? Thanks!
> 
> efi_rs_enter() and efi_rs_leave() look very PV x86 specific. It is
> possible that we don't actually need to do anything at all on ARM, aside
> from refactoring the code. Jan?

Whether you need to do much on ARM I don't know; doing
nothing doesn't seem to be among the options though, as at the
very least you need to acquire/release efi_rs_lock. The fact that
the code will need refactoring has been well known since the time
it got made common.

> Similarly we don't have an rtc_lock spinlock today and it is not
> actually needed, but for the sake of keeping the common code common, I
> would just introduce rtc_lock on ARM too.

I think this would better be hidden behind an arch abstraction,
doing nothing on ARM.

Jan