Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-20 Thread Julien Grall

Hi,

On 18/04/17 19:51, Juergen Gross wrote:

On 18/04/17 20:46, Stefano Stabellini wrote:

On Tue, 18 Apr 2017, Juergen Gross wrote:

On 18/04/17 20:37, Stefano Stabellini wrote:

On Thu, 6 Apr 2017, Juergen Gross wrote:

On 06/04/17 18:43, Daniel Kiper wrote:

On Thu, Apr 06, 2017 at 06:22:44PM +0200, Juergen Gross wrote:

On 06/04/17 18:06, Daniel Kiper wrote:

Hi Julien,

On Thu, Apr 06, 2017 at 04:39:13PM +0100, Julien Grall wrote:

Hi Daniel,

On 06/04/17 16:20, Daniel Kiper wrote:

On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:

On 06/04/17 16:27, Daniel Kiper wrote:

On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:

Hi Juergen,

On 06/04/17 07:23, Juergen Gross wrote:

On 05/04/17 21:49, Boris Ostrovsky wrote:

On 04/05/2017 02:14 PM, Julien Grall wrote:

The x86 code has theoritically a similar issue, altought EFI does not
seem to be the preferred method. I have left it unimplemented on x86 and
CCed Linux Xen x86 maintainers to know their view here.


(+Daniel)

This could be a problem for x86 as well, at least theoretically.
xen_machine_power_off() may call pm_power_off(), which is efi.reset_system.

So I think we should have a similar routine there.


+1

I don't see any problem with such a routine added, in contrast to
potential "reboots" instead of power off without it.

So I think this dummy xen_efi_reset_system() should be added to
drivers/xen/efi.c instead.


I will resend the patch during day with xen_efi_reset_system moved
to common code and implement the x86 counterpart (thought, I will
not be able to test it).


I think that this is ARM specific issue. On x86 machine_restart() calls
xen_restart(). Hence, everything works. So, I think that it should be
fixed only for ARM. Anyway, please CC me when you send a patch.


What about xen_machine_power_off() (as stated in Boris' mail)?


Guys what do you think about that:

--- a/drivers/firmware/efi/reboot.c
+++ b/drivers/firmware/efi/reboot.c
@@ -55,7 +55,7 @@ static void efi_power_off(void)

static int __init efi_shutdown_init(void)
{
-   if (!efi_enabled(EFI_RUNTIME_SERVICES))
+   if (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_PARAVIRT))
   return -ENODEV;

   if (efi_poweroff_required())


Julien, for ARM64 please take a look at 
arch/arm64/kernel/efi.c:efi_poweroff_required(void).

I hope that tweaks for both files should solve our problem.


This sounds good for power off (I haven't tried to power off DOM0
yet). But this will not solve the restart problem (see
machine_restart in arch/arm64/kernel/process.c) which call directly
efi_reboot.


Hmmm... It seems to me that efi.reset_system override with empty function
in arch/arm/xen/efi.c is the best solution. So, I see three patches here.
One for drivers/firmware/efi/reboot.c, one for arch/arm/xen/efi.c and one
for arch/arm64/kernel/efi.c. Does it make sense?


I still think the empty function should be in drivers/xen/efi.c and we
should use it in arch/x86/xen/efi.c, too.


If you wish we can go that way too. Though I thing that we should fix
drivers/firmware/efi/reboot.c:efi_shutdown_init() too. Just in case.


Sure, go ahead. I won't object.


For the Xen on ARM side, the original patch that started this thread
(20170405181417.15985-1-julien.gr...@arm.com) is good to go, right?



As I said: the dummy xen_efi_reset_system() should be in
drivers/xen/efi.c


OK. Who is working on it?


Didn't Julien say he would do it?


Yes. I looked at bit closer to the problem mention with power off. 
xen_efi_reset_system cannot be a NOP because there may not be fallback 
alternatives (see machine_power_off in arch/arm64/kernel/process.c)


So I think we would have to translate EFI_RESET* to Xen SHUTDOWN_* and 
then call HYPERVISOR_sched_op directly.


I will send a new version soon.

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-19 Thread Daniel Kiper
On Wed, Apr 19, 2017 at 08:37:38PM +0100, Matt Fleming wrote:
> On Wed, 19 Apr, at 09:29:06PM, Daniel Kiper wrote:
> > On Tue, Apr 18, 2017 at 02:46:50PM +0100, Matt Fleming wrote:
> > > On Thu, 06 Apr, at 04:55:11PM, Mark Rutland wrote:
> > > >
> > > > Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper,
> > > > rather than spreading it further.
> > > >
> > > > IMO, given reset_system is a *mandatory* function, the Xen wrapper
> > > > should provide an implementation.
> > > >
> > > > I don't see why you can't implement a wrapper that calls the usual Xen
> > > > poweroff/reset functions.
> > >
> > > I realise I'm making a sweeping generalisation, but adding
> > > EFI_PARAVIRT is almost always the wrong thing to do.
> >
> > Why?
>
> Because it makes paravirt a special case, and there's usually very
> little reason to make it special in the EFI code. Special-casing means
> more branches, more code paths, a bigger testing matrix and more
> complex code.
>
> EFI_PARAVIRT does have its uses, like for those scenarios where we
> don't have a table of function pointers that can be overidden for
> paravirt.
>
> But we do have such a table for ->reset_system().

This is more or less what I expected. Thanks a lot for explanation.

Daniel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-19 Thread Matt Fleming
On Wed, 19 Apr, at 09:29:06PM, Daniel Kiper wrote:
> On Tue, Apr 18, 2017 at 02:46:50PM +0100, Matt Fleming wrote:
> > On Thu, 06 Apr, at 04:55:11PM, Mark Rutland wrote:
> > >
> > > Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper,
> > > rather than spreading it further.
> > >
> > > IMO, given reset_system is a *mandatory* function, the Xen wrapper
> > > should provide an implementation.
> > >
> > > I don't see why you can't implement a wrapper that calls the usual Xen
> > > poweroff/reset functions.
> >
> > I realise I'm making a sweeping generalisation, but adding
> > EFI_PARAVIRT is almost always the wrong thing to do.
> 
> Why?
 
Because it makes paravirt a special case, and there's usually very
little reason to make it special in the EFI code. Special-casing means
more branches, more code paths, a bigger testing matrix and more
complex code. 

EFI_PARAVIRT does have its uses, like for those scenarios where we
don't have a table of function pointers that can be overidden for
paravirt.

But we do have such a table for ->reset_system().

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-19 Thread Daniel Kiper
On Tue, Apr 18, 2017 at 02:46:50PM +0100, Matt Fleming wrote:
> On Thu, 06 Apr, at 04:55:11PM, Mark Rutland wrote:
> >
> > Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper,
> > rather than spreading it further.
> >
> > IMO, given reset_system is a *mandatory* function, the Xen wrapper
> > should provide an implementation.
> >
> > I don't see why you can't implement a wrapper that calls the usual Xen
> > poweroff/reset functions.
>
> I realise I'm making a sweeping generalisation, but adding
> EFI_PARAVIRT is almost always the wrong thing to do.

Why?

> I'd much prefer to see Mark's idea implemented.

If you wish I do not object.

Daniel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-18 Thread Juergen Gross
On 18/04/17 20:46, Stefano Stabellini wrote:
> On Tue, 18 Apr 2017, Juergen Gross wrote:
>> On 18/04/17 20:37, Stefano Stabellini wrote:
>>> On Thu, 6 Apr 2017, Juergen Gross wrote:
 On 06/04/17 18:43, Daniel Kiper wrote:
> On Thu, Apr 06, 2017 at 06:22:44PM +0200, Juergen Gross wrote:
>> On 06/04/17 18:06, Daniel Kiper wrote:
>>> Hi Julien,
>>>
>>> On Thu, Apr 06, 2017 at 04:39:13PM +0100, Julien Grall wrote:
 Hi Daniel,

 On 06/04/17 16:20, Daniel Kiper wrote:
> On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
>> On 06/04/17 16:27, Daniel Kiper wrote:
>>> On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
 Hi Juergen,

 On 06/04/17 07:23, Juergen Gross wrote:
> On 05/04/17 21:49, Boris Ostrovsky wrote:
>> On 04/05/2017 02:14 PM, Julien Grall wrote:
>>> The x86 code has theoritically a similar issue, altought EFI 
>>> does not
>>> seem to be the preferred method. I have left it unimplemented 
>>> on x86 and
>>> CCed Linux Xen x86 maintainers to know their view here.
>>
>> (+Daniel)
>>
>> This could be a problem for x86 as well, at least theoretically.
>> xen_machine_power_off() may call pm_power_off(), which is 
>> efi.reset_system.
>>
>> So I think we should have a similar routine there.
>
> +1
>
> I don't see any problem with such a routine added, in contrast to
> potential "reboots" instead of power off without it.
>
> So I think this dummy xen_efi_reset_system() should be added to
> drivers/xen/efi.c instead.

 I will resend the patch during day with xen_efi_reset_system moved
 to common code and implement the x86 counterpart (thought, I will
 not be able to test it).
>>>
>>> I think that this is ARM specific issue. On x86 machine_restart() 
>>> calls
>>> xen_restart(). Hence, everything works. So, I think that it should 
>>> be
>>> fixed only for ARM. Anyway, please CC me when you send a patch.
>>
>> What about xen_machine_power_off() (as stated in Boris' mail)?
>
> Guys what do you think about that:
>
> --- a/drivers/firmware/efi/reboot.c
> +++ b/drivers/firmware/efi/reboot.c
> @@ -55,7 +55,7 @@ static void efi_power_off(void)
>
> static int __init efi_shutdown_init(void)
> {
> -   if (!efi_enabled(EFI_RUNTIME_SERVICES))
> +   if (!efi_enabled(EFI_RUNTIME_SERVICES) || 
> efi_enabled(EFI_PARAVIRT))
>return -ENODEV;
>
>if (efi_poweroff_required())
>
>
> Julien, for ARM64 please take a look at 
> arch/arm64/kernel/efi.c:efi_poweroff_required(void).
>
> I hope that tweaks for both files should solve our problem.

 This sounds good for power off (I haven't tried to power off DOM0
 yet). But this will not solve the restart problem (see
 machine_restart in arch/arm64/kernel/process.c) which call directly
 efi_reboot.
>>>
>>> Hmmm... It seems to me that efi.reset_system override with empty 
>>> function
>>> in arch/arm/xen/efi.c is the best solution. So, I see three patches 
>>> here.
>>> One for drivers/firmware/efi/reboot.c, one for arch/arm/xen/efi.c and 
>>> one
>>> for arch/arm64/kernel/efi.c. Does it make sense?
>>
>> I still think the empty function should be in drivers/xen/efi.c and we
>> should use it in arch/x86/xen/efi.c, too.
>
> If you wish we can go that way too. Though I thing that we should fix
> drivers/firmware/efi/reboot.c:efi_shutdown_init() too. Just in case.

 Sure, go ahead. I won't object.
>>>
>>> For the Xen on ARM side, the original patch that started this thread
>>> (20170405181417.15985-1-julien.gr...@arm.com) is good to go, right?
>>>
>>
>> As I said: the dummy xen_efi_reset_system() should be in
>> drivers/xen/efi.c
> 
> OK. Who is working on it?

Didn't Julien say he would do it?


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-18 Thread Stefano Stabellini
On Tue, 18 Apr 2017, Juergen Gross wrote:
> On 18/04/17 20:37, Stefano Stabellini wrote:
> > On Thu, 6 Apr 2017, Juergen Gross wrote:
> >> On 06/04/17 18:43, Daniel Kiper wrote:
> >>> On Thu, Apr 06, 2017 at 06:22:44PM +0200, Juergen Gross wrote:
>  On 06/04/17 18:06, Daniel Kiper wrote:
> > Hi Julien,
> >
> > On Thu, Apr 06, 2017 at 04:39:13PM +0100, Julien Grall wrote:
> >> Hi Daniel,
> >>
> >> On 06/04/17 16:20, Daniel Kiper wrote:
> >>> On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
>  On 06/04/17 16:27, Daniel Kiper wrote:
> > On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
> >> Hi Juergen,
> >>
> >> On 06/04/17 07:23, Juergen Gross wrote:
> >>> On 05/04/17 21:49, Boris Ostrovsky wrote:
>  On 04/05/2017 02:14 PM, Julien Grall wrote:
> > The x86 code has theoritically a similar issue, altought EFI 
> > does not
> > seem to be the preferred method. I have left it unimplemented 
> > on x86 and
> > CCed Linux Xen x86 maintainers to know their view here.
> 
>  (+Daniel)
> 
>  This could be a problem for x86 as well, at least theoretically.
>  xen_machine_power_off() may call pm_power_off(), which is 
>  efi.reset_system.
> 
>  So I think we should have a similar routine there.
> >>>
> >>> +1
> >>>
> >>> I don't see any problem with such a routine added, in contrast to
> >>> potential "reboots" instead of power off without it.
> >>>
> >>> So I think this dummy xen_efi_reset_system() should be added to
> >>> drivers/xen/efi.c instead.
> >>
> >> I will resend the patch during day with xen_efi_reset_system moved
> >> to common code and implement the x86 counterpart (thought, I will
> >> not be able to test it).
> >
> > I think that this is ARM specific issue. On x86 machine_restart() 
> > calls
> > xen_restart(). Hence, everything works. So, I think that it should 
> > be
> > fixed only for ARM. Anyway, please CC me when you send a patch.
> 
>  What about xen_machine_power_off() (as stated in Boris' mail)?
> >>>
> >>> Guys what do you think about that:
> >>>
> >>> --- a/drivers/firmware/efi/reboot.c
> >>> +++ b/drivers/firmware/efi/reboot.c
> >>> @@ -55,7 +55,7 @@ static void efi_power_off(void)
> >>>
> >>> static int __init efi_shutdown_init(void)
> >>> {
> >>> -   if (!efi_enabled(EFI_RUNTIME_SERVICES))
> >>> +   if (!efi_enabled(EFI_RUNTIME_SERVICES) || 
> >>> efi_enabled(EFI_PARAVIRT))
> >>>return -ENODEV;
> >>>
> >>>if (efi_poweroff_required())
> >>>
> >>>
> >>> Julien, for ARM64 please take a look at 
> >>> arch/arm64/kernel/efi.c:efi_poweroff_required(void).
> >>>
> >>> I hope that tweaks for both files should solve our problem.
> >>
> >> This sounds good for power off (I haven't tried to power off DOM0
> >> yet). But this will not solve the restart problem (see
> >> machine_restart in arch/arm64/kernel/process.c) which call directly
> >> efi_reboot.
> >
> > Hmmm... It seems to me that efi.reset_system override with empty 
> > function
> > in arch/arm/xen/efi.c is the best solution. So, I see three patches 
> > here.
> > One for drivers/firmware/efi/reboot.c, one for arch/arm/xen/efi.c and 
> > one
> > for arch/arm64/kernel/efi.c. Does it make sense?
> 
>  I still think the empty function should be in drivers/xen/efi.c and we
>  should use it in arch/x86/xen/efi.c, too.
> >>>
> >>> If you wish we can go that way too. Though I thing that we should fix
> >>> drivers/firmware/efi/reboot.c:efi_shutdown_init() too. Just in case.
> >>
> >> Sure, go ahead. I won't object.
> > 
> > For the Xen on ARM side, the original patch that started this thread
> > (20170405181417.15985-1-julien.gr...@arm.com) is good to go, right?
> > 
> 
> As I said: the dummy xen_efi_reset_system() should be in
> drivers/xen/efi.c

OK. Who is working on it?

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-18 Thread Juergen Gross
On 18/04/17 20:37, Stefano Stabellini wrote:
> On Thu, 6 Apr 2017, Juergen Gross wrote:
>> On 06/04/17 18:43, Daniel Kiper wrote:
>>> On Thu, Apr 06, 2017 at 06:22:44PM +0200, Juergen Gross wrote:
 On 06/04/17 18:06, Daniel Kiper wrote:
> Hi Julien,
>
> On Thu, Apr 06, 2017 at 04:39:13PM +0100, Julien Grall wrote:
>> Hi Daniel,
>>
>> On 06/04/17 16:20, Daniel Kiper wrote:
>>> On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
 On 06/04/17 16:27, Daniel Kiper wrote:
> On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
>> Hi Juergen,
>>
>> On 06/04/17 07:23, Juergen Gross wrote:
>>> On 05/04/17 21:49, Boris Ostrovsky wrote:
 On 04/05/2017 02:14 PM, Julien Grall wrote:
> The x86 code has theoritically a similar issue, altought EFI does 
> not
> seem to be the preferred method. I have left it unimplemented on 
> x86 and
> CCed Linux Xen x86 maintainers to know their view here.

 (+Daniel)

 This could be a problem for x86 as well, at least theoretically.
 xen_machine_power_off() may call pm_power_off(), which is 
 efi.reset_system.

 So I think we should have a similar routine there.
>>>
>>> +1
>>>
>>> I don't see any problem with such a routine added, in contrast to
>>> potential "reboots" instead of power off without it.
>>>
>>> So I think this dummy xen_efi_reset_system() should be added to
>>> drivers/xen/efi.c instead.
>>
>> I will resend the patch during day with xen_efi_reset_system moved
>> to common code and implement the x86 counterpart (thought, I will
>> not be able to test it).
>
> I think that this is ARM specific issue. On x86 machine_restart() 
> calls
> xen_restart(). Hence, everything works. So, I think that it should be
> fixed only for ARM. Anyway, please CC me when you send a patch.

 What about xen_machine_power_off() (as stated in Boris' mail)?
>>>
>>> Guys what do you think about that:
>>>
>>> --- a/drivers/firmware/efi/reboot.c
>>> +++ b/drivers/firmware/efi/reboot.c
>>> @@ -55,7 +55,7 @@ static void efi_power_off(void)
>>>
>>> static int __init efi_shutdown_init(void)
>>> {
>>> -   if (!efi_enabled(EFI_RUNTIME_SERVICES))
>>> +   if (!efi_enabled(EFI_RUNTIME_SERVICES) || 
>>> efi_enabled(EFI_PARAVIRT))
>>>return -ENODEV;
>>>
>>>if (efi_poweroff_required())
>>>
>>>
>>> Julien, for ARM64 please take a look at 
>>> arch/arm64/kernel/efi.c:efi_poweroff_required(void).
>>>
>>> I hope that tweaks for both files should solve our problem.
>>
>> This sounds good for power off (I haven't tried to power off DOM0
>> yet). But this will not solve the restart problem (see
>> machine_restart in arch/arm64/kernel/process.c) which call directly
>> efi_reboot.
>
> Hmmm... It seems to me that efi.reset_system override with empty function
> in arch/arm/xen/efi.c is the best solution. So, I see three patches here.
> One for drivers/firmware/efi/reboot.c, one for arch/arm/xen/efi.c and one
> for arch/arm64/kernel/efi.c. Does it make sense?

 I still think the empty function should be in drivers/xen/efi.c and we
 should use it in arch/x86/xen/efi.c, too.
>>>
>>> If you wish we can go that way too. Though I thing that we should fix
>>> drivers/firmware/efi/reboot.c:efi_shutdown_init() too. Just in case.
>>
>> Sure, go ahead. I won't object.
> 
> For the Xen on ARM side, the original patch that started this thread
> (20170405181417.15985-1-julien.gr...@arm.com) is good to go, right?
> 

As I said: the dummy xen_efi_reset_system() should be in
drivers/xen/efi.c


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-18 Thread Stefano Stabellini
On Thu, 6 Apr 2017, Juergen Gross wrote:
> On 06/04/17 18:43, Daniel Kiper wrote:
> > On Thu, Apr 06, 2017 at 06:22:44PM +0200, Juergen Gross wrote:
> >> On 06/04/17 18:06, Daniel Kiper wrote:
> >>> Hi Julien,
> >>>
> >>> On Thu, Apr 06, 2017 at 04:39:13PM +0100, Julien Grall wrote:
>  Hi Daniel,
> 
>  On 06/04/17 16:20, Daniel Kiper wrote:
> > On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
> >> On 06/04/17 16:27, Daniel Kiper wrote:
> >>> On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
>  Hi Juergen,
> 
>  On 06/04/17 07:23, Juergen Gross wrote:
> > On 05/04/17 21:49, Boris Ostrovsky wrote:
> >> On 04/05/2017 02:14 PM, Julien Grall wrote:
> >>> The x86 code has theoritically a similar issue, altought EFI does 
> >>> not
> >>> seem to be the preferred method. I have left it unimplemented on 
> >>> x86 and
> >>> CCed Linux Xen x86 maintainers to know their view here.
> >>
> >> (+Daniel)
> >>
> >> This could be a problem for x86 as well, at least theoretically.
> >> xen_machine_power_off() may call pm_power_off(), which is 
> >> efi.reset_system.
> >>
> >> So I think we should have a similar routine there.
> >
> > +1
> >
> > I don't see any problem with such a routine added, in contrast to
> > potential "reboots" instead of power off without it.
> >
> > So I think this dummy xen_efi_reset_system() should be added to
> > drivers/xen/efi.c instead.
> 
>  I will resend the patch during day with xen_efi_reset_system moved
>  to common code and implement the x86 counterpart (thought, I will
>  not be able to test it).
> >>>
> >>> I think that this is ARM specific issue. On x86 machine_restart() 
> >>> calls
> >>> xen_restart(). Hence, everything works. So, I think that it should be
> >>> fixed only for ARM. Anyway, please CC me when you send a patch.
> >>
> >> What about xen_machine_power_off() (as stated in Boris' mail)?
> >
> > Guys what do you think about that:
> >
> > --- a/drivers/firmware/efi/reboot.c
> > +++ b/drivers/firmware/efi/reboot.c
> > @@ -55,7 +55,7 @@ static void efi_power_off(void)
> >
> > static int __init efi_shutdown_init(void)
> > {
> > -   if (!efi_enabled(EFI_RUNTIME_SERVICES))
> > +   if (!efi_enabled(EFI_RUNTIME_SERVICES) || 
> > efi_enabled(EFI_PARAVIRT))
> >return -ENODEV;
> >
> >if (efi_poweroff_required())
> >
> >
> > Julien, for ARM64 please take a look at 
> > arch/arm64/kernel/efi.c:efi_poweroff_required(void).
> >
> > I hope that tweaks for both files should solve our problem.
> 
>  This sounds good for power off (I haven't tried to power off DOM0
>  yet). But this will not solve the restart problem (see
>  machine_restart in arch/arm64/kernel/process.c) which call directly
>  efi_reboot.
> >>>
> >>> Hmmm... It seems to me that efi.reset_system override with empty function
> >>> in arch/arm/xen/efi.c is the best solution. So, I see three patches here.
> >>> One for drivers/firmware/efi/reboot.c, one for arch/arm/xen/efi.c and one
> >>> for arch/arm64/kernel/efi.c. Does it make sense?
> >>
> >> I still think the empty function should be in drivers/xen/efi.c and we
> >> should use it in arch/x86/xen/efi.c, too.
> > 
> > If you wish we can go that way too. Though I thing that we should fix
> > drivers/firmware/efi/reboot.c:efi_shutdown_init() too. Just in case.
> 
> Sure, go ahead. I won't object.

For the Xen on ARM side, the original patch that started this thread
(20170405181417.15985-1-julien.gr...@arm.com) is good to go, right?

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-18 Thread Matt Fleming
On Thu, 06 Apr, at 04:55:11PM, Mark Rutland wrote:
> 
> Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper,
> rather than spreading it further.
> 
> IMO, given reset_system is a *mandatory* function, the Xen wrapper
> should provide an implementation.
> 
> I don't see why you can't implement a wrapper that calls the usual Xen
> poweroff/reset functions.

I realise I'm making a sweeping generalisation, but adding
EFI_PARAVIRT is almost always the wrong thing to do.

I'd much prefer to see Mark's idea implemented.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Juergen Gross
On 06/04/17 18:43, Daniel Kiper wrote:
> On Thu, Apr 06, 2017 at 06:22:44PM +0200, Juergen Gross wrote:
>> On 06/04/17 18:06, Daniel Kiper wrote:
>>> Hi Julien,
>>>
>>> On Thu, Apr 06, 2017 at 04:39:13PM +0100, Julien Grall wrote:
 Hi Daniel,

 On 06/04/17 16:20, Daniel Kiper wrote:
> On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
>> On 06/04/17 16:27, Daniel Kiper wrote:
>>> On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
 Hi Juergen,

 On 06/04/17 07:23, Juergen Gross wrote:
> On 05/04/17 21:49, Boris Ostrovsky wrote:
>> On 04/05/2017 02:14 PM, Julien Grall wrote:
>>> The x86 code has theoritically a similar issue, altought EFI does 
>>> not
>>> seem to be the preferred method. I have left it unimplemented on 
>>> x86 and
>>> CCed Linux Xen x86 maintainers to know their view here.
>>
>> (+Daniel)
>>
>> This could be a problem for x86 as well, at least theoretically.
>> xen_machine_power_off() may call pm_power_off(), which is 
>> efi.reset_system.
>>
>> So I think we should have a similar routine there.
>
> +1
>
> I don't see any problem with such a routine added, in contrast to
> potential "reboots" instead of power off without it.
>
> So I think this dummy xen_efi_reset_system() should be added to
> drivers/xen/efi.c instead.

 I will resend the patch during day with xen_efi_reset_system moved
 to common code and implement the x86 counterpart (thought, I will
 not be able to test it).
>>>
>>> I think that this is ARM specific issue. On x86 machine_restart() calls
>>> xen_restart(). Hence, everything works. So, I think that it should be
>>> fixed only for ARM. Anyway, please CC me when you send a patch.
>>
>> What about xen_machine_power_off() (as stated in Boris' mail)?
>
> Guys what do you think about that:
>
> --- a/drivers/firmware/efi/reboot.c
> +++ b/drivers/firmware/efi/reboot.c
> @@ -55,7 +55,7 @@ static void efi_power_off(void)
>
> static int __init efi_shutdown_init(void)
> {
> -   if (!efi_enabled(EFI_RUNTIME_SERVICES))
> +   if (!efi_enabled(EFI_RUNTIME_SERVICES) || 
> efi_enabled(EFI_PARAVIRT))
>return -ENODEV;
>
>if (efi_poweroff_required())
>
>
> Julien, for ARM64 please take a look at 
> arch/arm64/kernel/efi.c:efi_poweroff_required(void).
>
> I hope that tweaks for both files should solve our problem.

 This sounds good for power off (I haven't tried to power off DOM0
 yet). But this will not solve the restart problem (see
 machine_restart in arch/arm64/kernel/process.c) which call directly
 efi_reboot.
>>>
>>> Hmmm... It seems to me that efi.reset_system override with empty function
>>> in arch/arm/xen/efi.c is the best solution. So, I see three patches here.
>>> One for drivers/firmware/efi/reboot.c, one for arch/arm/xen/efi.c and one
>>> for arch/arm64/kernel/efi.c. Does it make sense?
>>
>> I still think the empty function should be in drivers/xen/efi.c and we
>> should use it in arch/x86/xen/efi.c, too.
> 
> If you wish we can go that way too. Though I thing that we should fix
> drivers/firmware/efi/reboot.c:efi_shutdown_init() too. Just in case.

Sure, go ahead. I won't object.


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Daniel Kiper
On Thu, Apr 06, 2017 at 06:22:44PM +0200, Juergen Gross wrote:
> On 06/04/17 18:06, Daniel Kiper wrote:
> > Hi Julien,
> >
> > On Thu, Apr 06, 2017 at 04:39:13PM +0100, Julien Grall wrote:
> >> Hi Daniel,
> >>
> >> On 06/04/17 16:20, Daniel Kiper wrote:
> >>> On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
>  On 06/04/17 16:27, Daniel Kiper wrote:
> > On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
> >> Hi Juergen,
> >>
> >> On 06/04/17 07:23, Juergen Gross wrote:
> >>> On 05/04/17 21:49, Boris Ostrovsky wrote:
>  On 04/05/2017 02:14 PM, Julien Grall wrote:
> > The x86 code has theoritically a similar issue, altought EFI does 
> > not
> > seem to be the preferred method. I have left it unimplemented on 
> > x86 and
> > CCed Linux Xen x86 maintainers to know their view here.
> 
>  (+Daniel)
> 
>  This could be a problem for x86 as well, at least theoretically.
>  xen_machine_power_off() may call pm_power_off(), which is 
>  efi.reset_system.
> 
>  So I think we should have a similar routine there.
> >>>
> >>> +1
> >>>
> >>> I don't see any problem with such a routine added, in contrast to
> >>> potential "reboots" instead of power off without it.
> >>>
> >>> So I think this dummy xen_efi_reset_system() should be added to
> >>> drivers/xen/efi.c instead.
> >>
> >> I will resend the patch during day with xen_efi_reset_system moved
> >> to common code and implement the x86 counterpart (thought, I will
> >> not be able to test it).
> >
> > I think that this is ARM specific issue. On x86 machine_restart() calls
> > xen_restart(). Hence, everything works. So, I think that it should be
> > fixed only for ARM. Anyway, please CC me when you send a patch.
> 
>  What about xen_machine_power_off() (as stated in Boris' mail)?
> >>>
> >>> Guys what do you think about that:
> >>>
> >>> --- a/drivers/firmware/efi/reboot.c
> >>> +++ b/drivers/firmware/efi/reboot.c
> >>> @@ -55,7 +55,7 @@ static void efi_power_off(void)
> >>>
> >>> static int __init efi_shutdown_init(void)
> >>> {
> >>> -   if (!efi_enabled(EFI_RUNTIME_SERVICES))
> >>> +   if (!efi_enabled(EFI_RUNTIME_SERVICES) || 
> >>> efi_enabled(EFI_PARAVIRT))
> >>>return -ENODEV;
> >>>
> >>>if (efi_poweroff_required())
> >>>
> >>>
> >>> Julien, for ARM64 please take a look at 
> >>> arch/arm64/kernel/efi.c:efi_poweroff_required(void).
> >>>
> >>> I hope that tweaks for both files should solve our problem.
> >>
> >> This sounds good for power off (I haven't tried to power off DOM0
> >> yet). But this will not solve the restart problem (see
> >> machine_restart in arch/arm64/kernel/process.c) which call directly
> >> efi_reboot.
> >
> > Hmmm... It seems to me that efi.reset_system override with empty function
> > in arch/arm/xen/efi.c is the best solution. So, I see three patches here.
> > One for drivers/firmware/efi/reboot.c, one for arch/arm/xen/efi.c and one
> > for arch/arm64/kernel/efi.c. Does it make sense?
>
> I still think the empty function should be in drivers/xen/efi.c and we
> should use it in arch/x86/xen/efi.c, too.

If you wish we can go that way too. Though I thing that we should fix
drivers/firmware/efi/reboot.c:efi_shutdown_init() too. Just in case.

Daniel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Juergen Gross
On 06/04/17 18:06, Daniel Kiper wrote:
> Hi Julien,
> 
> On Thu, Apr 06, 2017 at 04:39:13PM +0100, Julien Grall wrote:
>> Hi Daniel,
>>
>> On 06/04/17 16:20, Daniel Kiper wrote:
>>> On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
 On 06/04/17 16:27, Daniel Kiper wrote:
> On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
>> Hi Juergen,
>>
>> On 06/04/17 07:23, Juergen Gross wrote:
>>> On 05/04/17 21:49, Boris Ostrovsky wrote:
 On 04/05/2017 02:14 PM, Julien Grall wrote:
> The x86 code has theoritically a similar issue, altought EFI does not
> seem to be the preferred method. I have left it unimplemented on x86 
> and
> CCed Linux Xen x86 maintainers to know their view here.

 (+Daniel)

 This could be a problem for x86 as well, at least theoretically.
 xen_machine_power_off() may call pm_power_off(), which is 
 efi.reset_system.

 So I think we should have a similar routine there.
>>>
>>> +1
>>>
>>> I don't see any problem with such a routine added, in contrast to
>>> potential "reboots" instead of power off without it.
>>>
>>> So I think this dummy xen_efi_reset_system() should be added to
>>> drivers/xen/efi.c instead.
>>
>> I will resend the patch during day with xen_efi_reset_system moved
>> to common code and implement the x86 counterpart (thought, I will
>> not be able to test it).
>
> I think that this is ARM specific issue. On x86 machine_restart() calls
> xen_restart(). Hence, everything works. So, I think that it should be
> fixed only for ARM. Anyway, please CC me when you send a patch.

 What about xen_machine_power_off() (as stated in Boris' mail)?
>>>
>>> Guys what do you think about that:
>>>
>>> --- a/drivers/firmware/efi/reboot.c
>>> +++ b/drivers/firmware/efi/reboot.c
>>> @@ -55,7 +55,7 @@ static void efi_power_off(void)
>>>
>>> static int __init efi_shutdown_init(void)
>>> {
>>> -   if (!efi_enabled(EFI_RUNTIME_SERVICES))
>>> +   if (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_PARAVIRT))
>>>return -ENODEV;
>>>
>>>if (efi_poweroff_required())
>>>
>>>
>>> Julien, for ARM64 please take a look at 
>>> arch/arm64/kernel/efi.c:efi_poweroff_required(void).
>>>
>>> I hope that tweaks for both files should solve our problem.
>>
>> This sounds good for power off (I haven't tried to power off DOM0
>> yet). But this will not solve the restart problem (see
>> machine_restart in arch/arm64/kernel/process.c) which call directly
>> efi_reboot.
> 
> Hmmm... It seems to me that efi.reset_system override with empty function
> in arch/arm/xen/efi.c is the best solution. So, I see three patches here.
> One for drivers/firmware/efi/reboot.c, one for arch/arm/xen/efi.c and one
> for arch/arm64/kernel/efi.c. Does it make sense?

I still think the empty function should be in drivers/xen/efi.c and we
should use it in arch/x86/xen/efi.c, too.


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Daniel Kiper
Hi Julien,

On Thu, Apr 06, 2017 at 04:39:13PM +0100, Julien Grall wrote:
> Hi Daniel,
>
> On 06/04/17 16:20, Daniel Kiper wrote:
> >On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
> >>On 06/04/17 16:27, Daniel Kiper wrote:
> >>>On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
> Hi Juergen,
> 
> On 06/04/17 07:23, Juergen Gross wrote:
> >On 05/04/17 21:49, Boris Ostrovsky wrote:
> >>On 04/05/2017 02:14 PM, Julien Grall wrote:
> >>>The x86 code has theoritically a similar issue, altought EFI does not
> >>>seem to be the preferred method. I have left it unimplemented on x86 
> >>>and
> >>>CCed Linux Xen x86 maintainers to know their view here.
> >>
> >>(+Daniel)
> >>
> >>This could be a problem for x86 as well, at least theoretically.
> >>xen_machine_power_off() may call pm_power_off(), which is 
> >>efi.reset_system.
> >>
> >>So I think we should have a similar routine there.
> >
> >+1
> >
> >I don't see any problem with such a routine added, in contrast to
> >potential "reboots" instead of power off without it.
> >
> >So I think this dummy xen_efi_reset_system() should be added to
> >drivers/xen/efi.c instead.
> 
> I will resend the patch during day with xen_efi_reset_system moved
> to common code and implement the x86 counterpart (thought, I will
> not be able to test it).
> >>>
> >>>I think that this is ARM specific issue. On x86 machine_restart() calls
> >>>xen_restart(). Hence, everything works. So, I think that it should be
> >>>fixed only for ARM. Anyway, please CC me when you send a patch.
> >>
> >>What about xen_machine_power_off() (as stated in Boris' mail)?
> >
> >Guys what do you think about that:
> >
> >--- a/drivers/firmware/efi/reboot.c
> >+++ b/drivers/firmware/efi/reboot.c
> >@@ -55,7 +55,7 @@ static void efi_power_off(void)
> >
> > static int __init efi_shutdown_init(void)
> > {
> >-   if (!efi_enabled(EFI_RUNTIME_SERVICES))
> >+   if (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_PARAVIRT))
> >return -ENODEV;
> >
> >if (efi_poweroff_required())
> >
> >
> >Julien, for ARM64 please take a look at 
> >arch/arm64/kernel/efi.c:efi_poweroff_required(void).
> >
> >I hope that tweaks for both files should solve our problem.
>
> This sounds good for power off (I haven't tried to power off DOM0
> yet). But this will not solve the restart problem (see
> machine_restart in arch/arm64/kernel/process.c) which call directly
> efi_reboot.

Hmmm... It seems to me that efi.reset_system override with empty function
in arch/arm/xen/efi.c is the best solution. So, I see three patches here.
One for drivers/firmware/efi/reboot.c, one for arch/arm/xen/efi.c and one
for arch/arm64/kernel/efi.c. Does it make sense?

Daniel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Mark Rutland
[Adding the EFI maintainers]

Tl;DR: Xen's EFI wrappery doesn't implement reset_system, so when
invoked on arm64 we get a NULL dereference.

On Thu, Apr 06, 2017 at 04:39:13PM +0100, Julien Grall wrote:
> On 06/04/17 16:20, Daniel Kiper wrote:
> >On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
> >>On 06/04/17 16:27, Daniel Kiper wrote:
> >>>On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
> Hi Juergen,
> 
> On 06/04/17 07:23, Juergen Gross wrote:
> >On 05/04/17 21:49, Boris Ostrovsky wrote:
> >>On 04/05/2017 02:14 PM, Julien Grall wrote:
> >>>The x86 code has theoritically a similar issue, altought EFI does not
> >>>seem to be the preferred method. I have left it unimplemented on x86 
> >>>and
> >>>CCed Linux Xen x86 maintainers to know their view here.
> >>
> >>(+Daniel)
> >>
> >>This could be a problem for x86 as well, at least theoretically.
> >>xen_machine_power_off() may call pm_power_off(), which is 
> >>efi.reset_system.
> >>
> >>So I think we should have a similar routine there.
> >
> >+1
> >
> >I don't see any problem with such a routine added, in contrast to
> >potential "reboots" instead of power off without it.
> >
> >So I think this dummy xen_efi_reset_system() should be added to
> >drivers/xen/efi.c instead.
> 
> I will resend the patch during day with xen_efi_reset_system moved
> to common code and implement the x86 counterpart (thought, I will
> not be able to test it).
> >>>
> >>>I think that this is ARM specific issue. On x86 machine_restart() calls
> >>>xen_restart(). Hence, everything works. So, I think that it should be
> >>>fixed only for ARM. Anyway, please CC me when you send a patch.
> >>
> >>What about xen_machine_power_off() (as stated in Boris' mail)?
> >
> >Guys what do you think about that:
> >
> >--- a/drivers/firmware/efi/reboot.c
> >+++ b/drivers/firmware/efi/reboot.c
> >@@ -55,7 +55,7 @@ static void efi_power_off(void)
> >
> > static int __init efi_shutdown_init(void)
> > {
> >-   if (!efi_enabled(EFI_RUNTIME_SERVICES))
> >+   if (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_PARAVIRT))
> >return -ENODEV;
> >
> >if (efi_poweroff_required())
> >
> >
> >Julien, for ARM64 please take a look at 
> >arch/arm64/kernel/efi.c:efi_poweroff_required(void).
> >
> >I hope that tweaks for both files should solve our problem.
> 
> This sounds good for power off (I haven't tried to power off DOM0
> yet).

Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper,
rather than spreading it further.

IMO, given reset_system is a *mandatory* function, the Xen wrapper
should provide an implementation.

I don't see why you can't implement a wrapper that calls the usual Xen
poweroff/reset functions.

Thanks,
Mark.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Julien Grall

Hi Daniel,

On 06/04/17 16:20, Daniel Kiper wrote:

On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:

On 06/04/17 16:27, Daniel Kiper wrote:

On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:

Hi Juergen,

On 06/04/17 07:23, Juergen Gross wrote:

On 05/04/17 21:49, Boris Ostrovsky wrote:

On 04/05/2017 02:14 PM, Julien Grall wrote:

The x86 code has theoritically a similar issue, altought EFI does not
seem to be the preferred method. I have left it unimplemented on x86 and
CCed Linux Xen x86 maintainers to know their view here.


(+Daniel)

This could be a problem for x86 as well, at least theoretically.
xen_machine_power_off() may call pm_power_off(), which is efi.reset_system.

So I think we should have a similar routine there.


+1

I don't see any problem with such a routine added, in contrast to
potential "reboots" instead of power off without it.

So I think this dummy xen_efi_reset_system() should be added to
drivers/xen/efi.c instead.


I will resend the patch during day with xen_efi_reset_system moved
to common code and implement the x86 counterpart (thought, I will
not be able to test it).


I think that this is ARM specific issue. On x86 machine_restart() calls
xen_restart(). Hence, everything works. So, I think that it should be
fixed only for ARM. Anyway, please CC me when you send a patch.


What about xen_machine_power_off() (as stated in Boris' mail)?


Guys what do you think about that:

--- a/drivers/firmware/efi/reboot.c
+++ b/drivers/firmware/efi/reboot.c
@@ -55,7 +55,7 @@ static void efi_power_off(void)

 static int __init efi_shutdown_init(void)
 {
-   if (!efi_enabled(EFI_RUNTIME_SERVICES))
+   if (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_PARAVIRT))
return -ENODEV;

if (efi_poweroff_required())


Julien, for ARM64 please take a look at 
arch/arm64/kernel/efi.c:efi_poweroff_required(void).

I hope that tweaks for both files should solve our problem.


This sounds good for power off (I haven't tried to power off DOM0 yet). 
But this will not solve the restart problem (see machine_restart in 
arch/arm64/kernel/process.c) which call directly efi_reboot.


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Daniel Kiper
On Thu, Apr 06, 2017 at 04:38:24PM +0200, Juergen Gross wrote:
> On 06/04/17 16:27, Daniel Kiper wrote:
> > On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
> >> Hi Juergen,
> >>
> >> On 06/04/17 07:23, Juergen Gross wrote:
> >>> On 05/04/17 21:49, Boris Ostrovsky wrote:
>  On 04/05/2017 02:14 PM, Julien Grall wrote:
> > The x86 code has theoritically a similar issue, altought EFI does not
> > seem to be the preferred method. I have left it unimplemented on x86 and
> > CCed Linux Xen x86 maintainers to know their view here.
> 
>  (+Daniel)
> 
>  This could be a problem for x86 as well, at least theoretically.
>  xen_machine_power_off() may call pm_power_off(), which is 
>  efi.reset_system.
> 
>  So I think we should have a similar routine there.
> >>>
> >>> +1
> >>>
> >>> I don't see any problem with such a routine added, in contrast to
> >>> potential "reboots" instead of power off without it.
> >>>
> >>> So I think this dummy xen_efi_reset_system() should be added to
> >>> drivers/xen/efi.c instead.
> >>
> >> I will resend the patch during day with xen_efi_reset_system moved
> >> to common code and implement the x86 counterpart (thought, I will
> >> not be able to test it).
> >
> > I think that this is ARM specific issue. On x86 machine_restart() calls
> > xen_restart(). Hence, everything works. So, I think that it should be
> > fixed only for ARM. Anyway, please CC me when you send a patch.
>
> What about xen_machine_power_off() (as stated in Boris' mail)?

Guys what do you think about that:

--- a/drivers/firmware/efi/reboot.c
+++ b/drivers/firmware/efi/reboot.c
@@ -55,7 +55,7 @@ static void efi_power_off(void)

 static int __init efi_shutdown_init(void)
 {
-   if (!efi_enabled(EFI_RUNTIME_SERVICES))
+   if (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_PARAVIRT))
return -ENODEV;

if (efi_poweroff_required())


Julien, for ARM64 please take a look at 
arch/arm64/kernel/efi.c:efi_poweroff_required(void).

I hope that tweaks for both files should solve our problem.

Daniel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Juergen Gross
On 06/04/17 16:27, Daniel Kiper wrote:
> On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
>> Hi Juergen,
>>
>> On 06/04/17 07:23, Juergen Gross wrote:
>>> On 05/04/17 21:49, Boris Ostrovsky wrote:
 On 04/05/2017 02:14 PM, Julien Grall wrote:
> The x86 code has theoritically a similar issue, altought EFI does not
> seem to be the preferred method. I have left it unimplemented on x86 and
> CCed Linux Xen x86 maintainers to know their view here.

 (+Daniel)

 This could be a problem for x86 as well, at least theoretically.
 xen_machine_power_off() may call pm_power_off(), which is efi.reset_system.

 So I think we should have a similar routine there.
>>>
>>> +1
>>>
>>> I don't see any problem with such a routine added, in contrast to
>>> potential "reboots" instead of power off without it.
>>>
>>> So I think this dummy xen_efi_reset_system() should be added to
>>> drivers/xen/efi.c instead.
>>
>> I will resend the patch during day with xen_efi_reset_system moved
>> to common code and implement the x86 counterpart (thought, I will
>> not be able to test it).
> 
> I think that this is ARM specific issue. On x86 machine_restart() calls
> xen_restart(). Hence, everything works. So, I think that it should be
> fixed only for ARM. Anyway, please CC me when you send a patch.

What about xen_machine_power_off() (as stated in Boris' mail)?


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Boris Ostrovsky
On 04/06/2017 10:32 AM, Julien Grall wrote:
> Hi Daniel,
>
> On 06/04/17 15:27, Daniel Kiper wrote:
>> On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
>>> Hi Juergen,
>>>
>>> On 06/04/17 07:23, Juergen Gross wrote:
 On 05/04/17 21:49, Boris Ostrovsky wrote:
> On 04/05/2017 02:14 PM, Julien Grall wrote:
>> The x86 code has theoritically a similar issue, altought EFI does
>> not
>> seem to be the preferred method. I have left it unimplemented on
>> x86 and
>> CCed Linux Xen x86 maintainers to know their view here.
>
> (+Daniel)
>
> This could be a problem for x86 as well, at least theoretically.
> xen_machine_power_off() may call pm_power_off(), which is
> efi.reset_system.
>
> So I think we should have a similar routine there.

 +1

 I don't see any problem with such a routine added, in contrast to
 potential "reboots" instead of power off without it.

 So I think this dummy xen_efi_reset_system() should be added to
 drivers/xen/efi.c instead.
>>>
>>> I will resend the patch during day with xen_efi_reset_system moved
>>> to common code and implement the x86 counterpart (thought, I will
>>> not be able to test it).
>>
>> I think that this is ARM specific issue. On x86 machine_restart() calls
>> xen_restart(). Hence, everything works. So, I think that it should be
>> fixed only for ARM. Anyway, please CC me when you send a patch.
>
> This thread already a fix for ARM64. So do I need to resend a patch
> with x86 fixed or not?

Yes please. Daniel is correct that we are safe with xen_restart().
However, we are not safe when machine_ops.power_off is called.

Thanks.
-boris


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Julien Grall

Hi Daniel,

On 06/04/17 15:27, Daniel Kiper wrote:

On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:

Hi Juergen,

On 06/04/17 07:23, Juergen Gross wrote:

On 05/04/17 21:49, Boris Ostrovsky wrote:

On 04/05/2017 02:14 PM, Julien Grall wrote:

The x86 code has theoritically a similar issue, altought EFI does not
seem to be the preferred method. I have left it unimplemented on x86 and
CCed Linux Xen x86 maintainers to know their view here.


(+Daniel)

This could be a problem for x86 as well, at least theoretically.
xen_machine_power_off() may call pm_power_off(), which is efi.reset_system.

So I think we should have a similar routine there.


+1

I don't see any problem with such a routine added, in contrast to
potential "reboots" instead of power off without it.

So I think this dummy xen_efi_reset_system() should be added to
drivers/xen/efi.c instead.


I will resend the patch during day with xen_efi_reset_system moved
to common code and implement the x86 counterpart (thought, I will
not be able to test it).


I think that this is ARM specific issue. On x86 machine_restart() calls
xen_restart(). Hence, everything works. So, I think that it should be
fixed only for ARM. Anyway, please CC me when you send a patch.


This thread already a fix for ARM64. So do I need to resend a patch with 
x86 fixed or not?


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Daniel Kiper
On Thu, Apr 06, 2017 at 09:32:32AM +0100, Julien Grall wrote:
> Hi Juergen,
>
> On 06/04/17 07:23, Juergen Gross wrote:
> >On 05/04/17 21:49, Boris Ostrovsky wrote:
> >>On 04/05/2017 02:14 PM, Julien Grall wrote:
> >>>The x86 code has theoritically a similar issue, altought EFI does not
> >>>seem to be the preferred method. I have left it unimplemented on x86 and
> >>>CCed Linux Xen x86 maintainers to know their view here.
> >>
> >>(+Daniel)
> >>
> >>This could be a problem for x86 as well, at least theoretically.
> >>xen_machine_power_off() may call pm_power_off(), which is efi.reset_system.
> >>
> >>So I think we should have a similar routine there.
> >
> >+1
> >
> >I don't see any problem with such a routine added, in contrast to
> >potential "reboots" instead of power off without it.
> >
> >So I think this dummy xen_efi_reset_system() should be added to
> >drivers/xen/efi.c instead.
>
> I will resend the patch during day with xen_efi_reset_system moved
> to common code and implement the x86 counterpart (thought, I will
> not be able to test it).

I think that this is ARM specific issue. On x86 machine_restart() calls
xen_restart(). Hence, everything works. So, I think that it should be
fixed only for ARM. Anyway, please CC me when you send a patch.

Daniel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Juergen Gross
On 06/04/17 10:32, Julien Grall wrote:
> Hi Juergen,
> 
> On 06/04/17 07:23, Juergen Gross wrote:
>> On 05/04/17 21:49, Boris Ostrovsky wrote:
>>> On 04/05/2017 02:14 PM, Julien Grall wrote:
 The x86 code has theoritically a similar issue, altought EFI does not
 seem to be the preferred method. I have left it unimplemented on x86
 and
 CCed Linux Xen x86 maintainers to know their view here.
>>>
>>> (+Daniel)
>>>
>>> This could be a problem for x86 as well, at least theoretically.
>>> xen_machine_power_off() may call pm_power_off(), which is
>>> efi.reset_system.
>>>
>>> So I think we should have a similar routine there.
>>
>> +1
>>
>> I don't see any problem with such a routine added, in contrast to
>> potential "reboots" instead of power off without it.
>>
>> So I think this dummy xen_efi_reset_system() should be added to
>> drivers/xen/efi.c instead.
> 
> I will resend the patch during day with xen_efi_reset_system moved to
> common code and implement the x86 counterpart (thought, I will not be
> able to test it).

I'm rather sure it isn't hit very often. Otherwise there would be more
complaints about crashes during power off (in fact I do remember several
occasions where somebody claimed power off seemed to do a reboot only).


Juergen

>>
 This should also probably be fixed in stable tree.
>>
>> Yes.
> 
> I will CC stable.
> 
> Thank you,
> 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Julien Grall

Hi Juergen,

On 06/04/17 07:23, Juergen Gross wrote:

On 05/04/17 21:49, Boris Ostrovsky wrote:

On 04/05/2017 02:14 PM, Julien Grall wrote:

The x86 code has theoritically a similar issue, altought EFI does not
seem to be the preferred method. I have left it unimplemented on x86 and
CCed Linux Xen x86 maintainers to know their view here.


(+Daniel)

This could be a problem for x86 as well, at least theoretically.
xen_machine_power_off() may call pm_power_off(), which is efi.reset_system.

So I think we should have a similar routine there.


+1

I don't see any problem with such a routine added, in contrast to
potential "reboots" instead of power off without it.

So I think this dummy xen_efi_reset_system() should be added to
drivers/xen/efi.c instead.


I will resend the patch during day with xen_efi_reset_system moved to 
common code and implement the x86 counterpart (thought, I will not be 
able to test it).





This should also probably be fixed in stable tree.


Yes.


I will CC stable.

Thank you,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-06 Thread Juergen Gross
On 05/04/17 21:49, Boris Ostrovsky wrote:
> On 04/05/2017 02:14 PM, Julien Grall wrote:
>> When rebooting DOM0 with ACPI, the kernel is crashing with the stack trace 
>> [1].
>>
>> This is happening because when EFI runtimes are enabled, the reset code
>> (see machin_restart) will first try to use EFI restart method.
>>
>> However, the EFI restart code is expecting the reset_system callback to
>> be always set. This is not the case for Xen and will lead to crash.
>>
>> Looking at the reboot path, it is expected to fallback on an alternative
>> reboot method if one does not work. So implement reset_system callback
>> as a NOP for Xen.
>>
>> [   36.999270] reboot: Restarting system
>> [   37.002921] Internal error: Attempting to execute userspace memory: 
>> 8604 [#1] PREEMPT SMP
>> [   37.011460] Modules linked in:
>> [   37.014598] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 
>> 4.11.0-rc1-3-g1e248b60a39b-dirty #506
>> [   37.023903] Hardware name: (null) (DT)
>> [   37.027734] task: 800902068000 task.stack: 800902064000
>> [   37.033739] PC is at 0x0
>> [   37.036359] LR is at efi_reboot+0x94/0xd0
>> [   37.040438] pc : [<>] lr : [] pstate: 
>> 404001c5
>> [   37.047920] sp : 800902067cf0
>> [   37.051314] x29: 800902067cf0 x28: 800902068000
>> [   37.056709] x27: 08992000 x26: 008e
>> [   37.062104] x25: 0123 x24: 0015
>> [   37.067499] x23:  x22: 08e6e250
>> [   37.072894] x21: 08e6e000 x20: 
>> [   37.078289] x19: 08e5d4c8 x18: 0010
>> [   37.083684] x17: a7c27470 x16: deadbeef
>> [   37.089079] x15: 0006 x14: 88f42bef
>> [   37.094474] x13: 08f42bfd x12: 08e706c0
>> [   37.099870] x11: 08e7 x10: 05f5e0ff
>> [   37.105265] x9 : 800902067a50 x8 : 6974726174736552
>> [   37.110660] x7 : 08cc6fb8 x6 : 08cc6fb0
>> [   37.116055] x5 : 08c97dd8 x4 : 
>> [   37.121453] x3 :  x2 : 
>> [   37.126845] x1 :  x0 : 
>> [   37.132239]
>> [   37.133808] Process systemd-shutdow (pid: 1, stack limit = 
>> 0x800902064000)
>> [   37.141118] Stack: (0x800902067cf0 to 0x800902068000)
>> [   37.146949] 7ce0:   800902067d40 
>> 08085334
>> [   37.154869] 7d00:  08f3b000 800902067d40 
>> 080852e0
>> [   37.162787] 7d20: 08cc6fb0 08cc6fb8 08c7f580 
>> 08c97dd8
>> [   37.170706] 7d40: 800902067d60 080e2c2c  
>> 01234567
>> [   37.178624] 7d60: 800902067d80 080e2ee8  
>> 080e2df4
>> [   37.186544] 7d80:  080830f0  
>> 8008ff1c1000
>> [   37.194462] 7da0:  a7c4b1cc  
>> 0024
>> [   37.202380] 7dc0: 800902067dd0 0005 f24743c8 
>> 0004
>> [   37.210299] 7de0: f2475f03 0010 f2474418 
>> 0005
>> [   37.218218] 7e00: f2474578 000a d6b722c0 
>> 0001
>> [   37.226136] 7e20: 0123 0038 800902067e50 
>> 081e7294
>> [   37.234055] 7e40: 800902067e60 081e935c 800902067e60 
>> 081e9388
>> [   37.241973] 7e60: 800902067eb0 081ea388  
>> 8008ff1c1000
>> [   37.249892] 7e80:  a7c4a79c  
>> 0002
>> [   37.257810] 7ea0: 0104   
>> 080830f0
>> [   37.265729] 7ec0: fee1dead 28121969 01234567 
>> 
>> [   37.273651] 7ee0:  80800080 80008080 
>> feffa9a9d4ff2d66
>> [   37.281567] 7f00: 008e feffa9a9d5b60e0f 7f7f7f7f 
>> 0101010101010101
>> [   37.289485] 7f20: 0010 0008 003a 
>> a7ccf588
>> [   37.297404] 7f40: d6b87d00 a7c4b1b0 f2474be0 
>> d6b88000
>> [   37.305326] 7f60: f2474fb0 01234567  
>> 
>> [   37.313240] 7f80:  0001 d6b70d4d 
>> 
>> [   37.321159] 7fa0: 0001 f2474ea0 d6b5e2e0 
>> f2474e80
>> [   37.329078] 7fc0: a7c4b1cc  fee1dead 
>> 008e
>> [   37.336997] 7fe0:   9ce839cffee77eab 
>> fafdbf9f7ed57f2f
>> [   37.344911] Call trace:
>> [   37.347437] Exception stack(0x800902067b20 to 0x800902067c50)
>> [   37.353970] 7b20: 08e5d4c8 0001 80f82000 
>> 
>> [   37.361883] 7b40: 800902067b60 

Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-05 Thread Stefano Stabellini
On Wed, 5 Apr 2017, Julien Grall wrote:
> When rebooting DOM0 with ACPI, the kernel is crashing with the stack trace 
> [1].
> 
> This is happening because when EFI runtimes are enabled, the reset code
> (see machin_restart) will first try to use EFI restart method.
> 
> However, the EFI restart code is expecting the reset_system callback to
> be always set. This is not the case for Xen and will lead to crash.
> 
> Looking at the reboot path, it is expected to fallback on an alternative
> reboot method if one does not work. So implement reset_system callback
> as a NOP for Xen.
> 
> [   36.999270] reboot: Restarting system
> [   37.002921] Internal error: Attempting to execute userspace memory: 
> 8604 [#1] PREEMPT SMP
> [   37.011460] Modules linked in:
> [   37.014598] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 
> 4.11.0-rc1-3-g1e248b60a39b-dirty #506
> [   37.023903] Hardware name: (null) (DT)
> [   37.027734] task: 800902068000 task.stack: 800902064000
> [   37.033739] PC is at 0x0
> [   37.036359] LR is at efi_reboot+0x94/0xd0
> [   37.040438] pc : [<>] lr : [] pstate: 
> 404001c5
> [   37.047920] sp : 800902067cf0
> [   37.051314] x29: 800902067cf0 x28: 800902068000
> [   37.056709] x27: 08992000 x26: 008e
> [   37.062104] x25: 0123 x24: 0015
> [   37.067499] x23:  x22: 08e6e250
> [   37.072894] x21: 08e6e000 x20: 
> [   37.078289] x19: 08e5d4c8 x18: 0010
> [   37.083684] x17: a7c27470 x16: deadbeef
> [   37.089079] x15: 0006 x14: 88f42bef
> [   37.094474] x13: 08f42bfd x12: 08e706c0
> [   37.099870] x11: 08e7 x10: 05f5e0ff
> [   37.105265] x9 : 800902067a50 x8 : 6974726174736552
> [   37.110660] x7 : 08cc6fb8 x6 : 08cc6fb0
> [   37.116055] x5 : 08c97dd8 x4 : 
> [   37.121453] x3 :  x2 : 
> [   37.126845] x1 :  x0 : 
> [   37.132239]
> [   37.133808] Process systemd-shutdow (pid: 1, stack limit = 
> 0x800902064000)
> [   37.141118] Stack: (0x800902067cf0 to 0x800902068000)
> [   37.146949] 7ce0:   800902067d40 
> 08085334
> [   37.154869] 7d00:  08f3b000 800902067d40 
> 080852e0
> [   37.162787] 7d20: 08cc6fb0 08cc6fb8 08c7f580 
> 08c97dd8
> [   37.170706] 7d40: 800902067d60 080e2c2c  
> 01234567
> [   37.178624] 7d60: 800902067d80 080e2ee8  
> 080e2df4
> [   37.186544] 7d80:  080830f0  
> 8008ff1c1000
> [   37.194462] 7da0:  a7c4b1cc  
> 0024
> [   37.202380] 7dc0: 800902067dd0 0005 f24743c8 
> 0004
> [   37.210299] 7de0: f2475f03 0010 f2474418 
> 0005
> [   37.218218] 7e00: f2474578 000a d6b722c0 
> 0001
> [   37.226136] 7e20: 0123 0038 800902067e50 
> 081e7294
> [   37.234055] 7e40: 800902067e60 081e935c 800902067e60 
> 081e9388
> [   37.241973] 7e60: 800902067eb0 081ea388  
> 8008ff1c1000
> [   37.249892] 7e80:  a7c4a79c  
> 0002
> [   37.257810] 7ea0: 0104   
> 080830f0
> [   37.265729] 7ec0: fee1dead 28121969 01234567 
> 
> [   37.273651] 7ee0:  80800080 80008080 
> feffa9a9d4ff2d66
> [   37.281567] 7f00: 008e feffa9a9d5b60e0f 7f7f7f7f 
> 0101010101010101
> [   37.289485] 7f20: 0010 0008 003a 
> a7ccf588
> [   37.297404] 7f40: d6b87d00 a7c4b1b0 f2474be0 
> d6b88000
> [   37.305326] 7f60: f2474fb0 01234567  
> 
> [   37.313240] 7f80:  0001 d6b70d4d 
> 
> [   37.321159] 7fa0: 0001 f2474ea0 d6b5e2e0 
> f2474e80
> [   37.329078] 7fc0: a7c4b1cc  fee1dead 
> 008e
> [   37.336997] 7fe0:   9ce839cffee77eab 
> fafdbf9f7ed57f2f
> [   37.344911] Call trace:
> [   37.347437] Exception stack(0x800902067b20 to 0x800902067c50)
> [   37.353970] 7b20: 08e5d4c8 0001 80f82000 
> 
> [   37.361883] 7b40: 800902067b60 08e17000 08f44c68 
> 0001081081b4
> [   37.369802] 7b60: 800902067bf0 08108478  
> 

Re: [Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-05 Thread Boris Ostrovsky
On 04/05/2017 02:14 PM, Julien Grall wrote:
> When rebooting DOM0 with ACPI, the kernel is crashing with the stack trace 
> [1].
>
> This is happening because when EFI runtimes are enabled, the reset code
> (see machin_restart) will first try to use EFI restart method.
>
> However, the EFI restart code is expecting the reset_system callback to
> be always set. This is not the case for Xen and will lead to crash.
>
> Looking at the reboot path, it is expected to fallback on an alternative
> reboot method if one does not work. So implement reset_system callback
> as a NOP for Xen.
>
> [   36.999270] reboot: Restarting system
> [   37.002921] Internal error: Attempting to execute userspace memory: 
> 8604 [#1] PREEMPT SMP
> [   37.011460] Modules linked in:
> [   37.014598] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 
> 4.11.0-rc1-3-g1e248b60a39b-dirty #506
> [   37.023903] Hardware name: (null) (DT)
> [   37.027734] task: 800902068000 task.stack: 800902064000
> [   37.033739] PC is at 0x0
> [   37.036359] LR is at efi_reboot+0x94/0xd0
> [   37.040438] pc : [<>] lr : [] pstate: 
> 404001c5
> [   37.047920] sp : 800902067cf0
> [   37.051314] x29: 800902067cf0 x28: 800902068000
> [   37.056709] x27: 08992000 x26: 008e
> [   37.062104] x25: 0123 x24: 0015
> [   37.067499] x23:  x22: 08e6e250
> [   37.072894] x21: 08e6e000 x20: 
> [   37.078289] x19: 08e5d4c8 x18: 0010
> [   37.083684] x17: a7c27470 x16: deadbeef
> [   37.089079] x15: 0006 x14: 88f42bef
> [   37.094474] x13: 08f42bfd x12: 08e706c0
> [   37.099870] x11: 08e7 x10: 05f5e0ff
> [   37.105265] x9 : 800902067a50 x8 : 6974726174736552
> [   37.110660] x7 : 08cc6fb8 x6 : 08cc6fb0
> [   37.116055] x5 : 08c97dd8 x4 : 
> [   37.121453] x3 :  x2 : 
> [   37.126845] x1 :  x0 : 
> [   37.132239]
> [   37.133808] Process systemd-shutdow (pid: 1, stack limit = 
> 0x800902064000)
> [   37.141118] Stack: (0x800902067cf0 to 0x800902068000)
> [   37.146949] 7ce0:   800902067d40 
> 08085334
> [   37.154869] 7d00:  08f3b000 800902067d40 
> 080852e0
> [   37.162787] 7d20: 08cc6fb0 08cc6fb8 08c7f580 
> 08c97dd8
> [   37.170706] 7d40: 800902067d60 080e2c2c  
> 01234567
> [   37.178624] 7d60: 800902067d80 080e2ee8  
> 080e2df4
> [   37.186544] 7d80:  080830f0  
> 8008ff1c1000
> [   37.194462] 7da0:  a7c4b1cc  
> 0024
> [   37.202380] 7dc0: 800902067dd0 0005 f24743c8 
> 0004
> [   37.210299] 7de0: f2475f03 0010 f2474418 
> 0005
> [   37.218218] 7e00: f2474578 000a d6b722c0 
> 0001
> [   37.226136] 7e20: 0123 0038 800902067e50 
> 081e7294
> [   37.234055] 7e40: 800902067e60 081e935c 800902067e60 
> 081e9388
> [   37.241973] 7e60: 800902067eb0 081ea388  
> 8008ff1c1000
> [   37.249892] 7e80:  a7c4a79c  
> 0002
> [   37.257810] 7ea0: 0104   
> 080830f0
> [   37.265729] 7ec0: fee1dead 28121969 01234567 
> 
> [   37.273651] 7ee0:  80800080 80008080 
> feffa9a9d4ff2d66
> [   37.281567] 7f00: 008e feffa9a9d5b60e0f 7f7f7f7f 
> 0101010101010101
> [   37.289485] 7f20: 0010 0008 003a 
> a7ccf588
> [   37.297404] 7f40: d6b87d00 a7c4b1b0 f2474be0 
> d6b88000
> [   37.305326] 7f60: f2474fb0 01234567  
> 
> [   37.313240] 7f80:  0001 d6b70d4d 
> 
> [   37.321159] 7fa0: 0001 f2474ea0 d6b5e2e0 
> f2474e80
> [   37.329078] 7fc0: a7c4b1cc  fee1dead 
> 008e
> [   37.336997] 7fe0:   9ce839cffee77eab 
> fafdbf9f7ed57f2f
> [   37.344911] Call trace:
> [   37.347437] Exception stack(0x800902067b20 to 0x800902067c50)
> [   37.353970] 7b20: 08e5d4c8 0001 80f82000 
> 
> [   37.361883] 7b40: 800902067b60 08e17000 08f44c68 
> 0001081081b4
> [   37.369802] 7b60: 800902067bf0 08108478  
> 

[Xen-devel] [PATCH] arm64: xen: Implement EFI reset_system callback

2017-04-05 Thread Julien Grall
When rebooting DOM0 with ACPI, the kernel is crashing with the stack trace [1].

This is happening because when EFI runtimes are enabled, the reset code
(see machin_restart) will first try to use EFI restart method.

However, the EFI restart code is expecting the reset_system callback to
be always set. This is not the case for Xen and will lead to crash.

Looking at the reboot path, it is expected to fallback on an alternative
reboot method if one does not work. So implement reset_system callback
as a NOP for Xen.

[   36.999270] reboot: Restarting system
[   37.002921] Internal error: Attempting to execute userspace memory: 8604 
[#1] PREEMPT SMP
[   37.011460] Modules linked in:
[   37.014598] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 
4.11.0-rc1-3-g1e248b60a39b-dirty #506
[   37.023903] Hardware name: (null) (DT)
[   37.027734] task: 800902068000 task.stack: 800902064000
[   37.033739] PC is at 0x0
[   37.036359] LR is at efi_reboot+0x94/0xd0
[   37.040438] pc : [<>] lr : [] pstate: 
404001c5
[   37.047920] sp : 800902067cf0
[   37.051314] x29: 800902067cf0 x28: 800902068000
[   37.056709] x27: 08992000 x26: 008e
[   37.062104] x25: 0123 x24: 0015
[   37.067499] x23:  x22: 08e6e250
[   37.072894] x21: 08e6e000 x20: 
[   37.078289] x19: 08e5d4c8 x18: 0010
[   37.083684] x17: a7c27470 x16: deadbeef
[   37.089079] x15: 0006 x14: 88f42bef
[   37.094474] x13: 08f42bfd x12: 08e706c0
[   37.099870] x11: 08e7 x10: 05f5e0ff
[   37.105265] x9 : 800902067a50 x8 : 6974726174736552
[   37.110660] x7 : 08cc6fb8 x6 : 08cc6fb0
[   37.116055] x5 : 08c97dd8 x4 : 
[   37.121453] x3 :  x2 : 
[   37.126845] x1 :  x0 : 
[   37.132239]
[   37.133808] Process systemd-shutdow (pid: 1, stack limit = 
0x800902064000)
[   37.141118] Stack: (0x800902067cf0 to 0x800902068000)
[   37.146949] 7ce0:   800902067d40 
08085334
[   37.154869] 7d00:  08f3b000 800902067d40 
080852e0
[   37.162787] 7d20: 08cc6fb0 08cc6fb8 08c7f580 
08c97dd8
[   37.170706] 7d40: 800902067d60 080e2c2c  
01234567
[   37.178624] 7d60: 800902067d80 080e2ee8  
080e2df4
[   37.186544] 7d80:  080830f0  
8008ff1c1000
[   37.194462] 7da0:  a7c4b1cc  
0024
[   37.202380] 7dc0: 800902067dd0 0005 f24743c8 
0004
[   37.210299] 7de0: f2475f03 0010 f2474418 
0005
[   37.218218] 7e00: f2474578 000a d6b722c0 
0001
[   37.226136] 7e20: 0123 0038 800902067e50 
081e7294
[   37.234055] 7e40: 800902067e60 081e935c 800902067e60 
081e9388
[   37.241973] 7e60: 800902067eb0 081ea388  
8008ff1c1000
[   37.249892] 7e80:  a7c4a79c  
0002
[   37.257810] 7ea0: 0104   
080830f0
[   37.265729] 7ec0: fee1dead 28121969 01234567 

[   37.273651] 7ee0:  80800080 80008080 
feffa9a9d4ff2d66
[   37.281567] 7f00: 008e feffa9a9d5b60e0f 7f7f7f7f 
0101010101010101
[   37.289485] 7f20: 0010 0008 003a 
a7ccf588
[   37.297404] 7f40: d6b87d00 a7c4b1b0 f2474be0 
d6b88000
[   37.305326] 7f60: f2474fb0 01234567  

[   37.313240] 7f80:  0001 d6b70d4d 

[   37.321159] 7fa0: 0001 f2474ea0 d6b5e2e0 
f2474e80
[   37.329078] 7fc0: a7c4b1cc  fee1dead 
008e
[   37.336997] 7fe0:   9ce839cffee77eab 
fafdbf9f7ed57f2f
[   37.344911] Call trace:
[   37.347437] Exception stack(0x800902067b20 to 0x800902067c50)
[   37.353970] 7b20: 08e5d4c8 0001 80f82000 

[   37.361883] 7b40: 800902067b60 08e17000 08f44c68 
0001081081b4
[   37.369802] 7b60: 800902067bf0 08108478  
08c235b0
[   37.377721] 7b80: 800902067ce0   
0015
[   37.385643] 7ba0: 0123 008e 08992000 
800902068000
[   37.393557] 7bc0: