All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-6.0] spapr: Allow memory unplug to always succeed
@ 2020-12-07 13:37 Greg Kurz
  2020-12-08  4:30 ` David Gibson
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kurz @ 2020-12-07 13:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, Greg Kurz, David Gibson

It is currently impossible to hot-unplug a memory device between
machine reset and CAS.

(qemu) device_del dimm1
Error: Memory hot unplug not supported for this guest

This limitation was introduced in order to provide an explicit
error path for older guests that didn't support hot-plug event
sources (and thus memory hot-unplug).

The linux kernel has been supporting these since 4.11. All recent
enough guests are thus capable of handling the removal of a memory
device at all time, including during early boot.

Lift the limitation for the latest machine type. This means that
trying to unplug memory from a guest that doesn't support it will
likely just do nothing and the memory will only get removed at
next reboot. Such older guests can still get the existing behavior
by using an older machine type.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
Based-on: 20201109173928.1001764-1-cohuck@redhat.com
---
 include/hw/ppc/spapr.h | 1 +
 hw/ppc/spapr.c         | 6 +++++-
 hw/ppc/spapr_events.c  | 3 ++-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index b7ced9faebf5..7aa630350326 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -139,6 +139,7 @@ struct SpaprMachineClass {
     hwaddr rma_limit;          /* clamp the RMA to this size */
     bool pre_5_1_assoc_refpoints;
     bool pre_5_2_numa_associativity;
+    bool pre_6_0_memory_unplug;
 
     bool (*phb_placement)(SpaprMachineState *spapr, uint32_t index,
                           uint64_t *buid, hwaddr *pio, 
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7e954bc84bed..f0b26b2af30d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4044,7 +4044,8 @@ static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
     SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
 
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
-        if (spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) {
+        if (!smc->pre_6_0_memory_unplug ||
+            spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) {
             spapr_memory_unplug_request(hotplug_dev, dev, errp);
         } else {
             /* NOTE: this means there is a window after guest reset, prior to
@@ -4530,8 +4531,11 @@ DEFINE_SPAPR_MACHINE(6_0, "6.0", true);
  */
 static void spapr_machine_5_2_class_options(MachineClass *mc)
 {
+    SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
+
     spapr_machine_6_0_class_options(mc);
     compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len);
+    smc->pre_6_0_memory_unplug = true;
 }
 
 DEFINE_SPAPR_MACHINE(5_2, "5.2", false);
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 1add53547ec3..c30123177b16 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -659,7 +659,8 @@ static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
         /* we should not be using count_indexed value unless the guest
          * supports dedicated hotplug event source
          */
-        g_assert(spapr_ovec_test(spapr->ov5_cas, OV5_HP_EVT));
+        g_assert(!SPAPR_MACHINE_GET_CLASS(spapr)->pre_6_0_memory_unplug ||
+                 spapr_ovec_test(spapr->ov5_cas, OV5_HP_EVT));
         hp->drc_id.count_indexed.count =
             cpu_to_be32(drc_id->count_indexed.count);
         hp->drc_id.count_indexed.index =
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH for-6.0] spapr: Allow memory unplug to always succeed
  2020-12-07 13:37 [PATCH for-6.0] spapr: Allow memory unplug to always succeed Greg Kurz
@ 2020-12-08  4:30 ` David Gibson
  2020-12-08  9:06   ` Greg Kurz
  0 siblings, 1 reply; 5+ messages in thread
From: David Gibson @ 2020-12-08  4:30 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 3965 bytes --]

On Mon, Dec 07, 2020 at 02:37:04PM +0100, Greg Kurz wrote:
> It is currently impossible to hot-unplug a memory device between
> machine reset and CAS.
> 
> (qemu) device_del dimm1
> Error: Memory hot unplug not supported for this guest
> 
> This limitation was introduced in order to provide an explicit
> error path for older guests that didn't support hot-plug event
> sources (and thus memory hot-unplug).
> 
> The linux kernel has been supporting these since 4.11. All recent
> enough guests are thus capable of handling the removal of a memory
> device at all time, including during early boot.
> 
> Lift the limitation for the latest machine type. This means that
> trying to unplug memory from a guest that doesn't support it will
> likely just do nothing and the memory will only get removed at
> next reboot. Such older guests can still get the existing behavior
> by using an older machine type.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Looks like this conflicts with something I've added to for-6.0
recently.  Can you rebase and resend, please.

> ---
> Based-on: 20201109173928.1001764-1-cohuck@redhat.com
> ---
>  include/hw/ppc/spapr.h | 1 +
>  hw/ppc/spapr.c         | 6 +++++-
>  hw/ppc/spapr_events.c  | 3 ++-
>  3 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index b7ced9faebf5..7aa630350326 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -139,6 +139,7 @@ struct SpaprMachineClass {
>      hwaddr rma_limit;          /* clamp the RMA to this size */
>      bool pre_5_1_assoc_refpoints;
>      bool pre_5_2_numa_associativity;
> +    bool pre_6_0_memory_unplug;
>  
>      bool (*phb_placement)(SpaprMachineState *spapr, uint32_t index,
>                            uint64_t *buid, hwaddr *pio, 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7e954bc84bed..f0b26b2af30d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4044,7 +4044,8 @@ static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
>      SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
>  
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> -        if (spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) {
> +        if (!smc->pre_6_0_memory_unplug ||
> +            spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) {
>              spapr_memory_unplug_request(hotplug_dev, dev, errp);
>          } else {
>              /* NOTE: this means there is a window after guest reset, prior to
> @@ -4530,8 +4531,11 @@ DEFINE_SPAPR_MACHINE(6_0, "6.0", true);
>   */
>  static void spapr_machine_5_2_class_options(MachineClass *mc)
>  {
> +    SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> +
>      spapr_machine_6_0_class_options(mc);
>      compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len);
> +    smc->pre_6_0_memory_unplug = true;
>  }
>  
>  DEFINE_SPAPR_MACHINE(5_2, "5.2", false);
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index 1add53547ec3..c30123177b16 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -659,7 +659,8 @@ static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
>          /* we should not be using count_indexed value unless the guest
>           * supports dedicated hotplug event source
>           */
> -        g_assert(spapr_ovec_test(spapr->ov5_cas, OV5_HP_EVT));
> +        g_assert(!SPAPR_MACHINE_GET_CLASS(spapr)->pre_6_0_memory_unplug ||
> +                 spapr_ovec_test(spapr->ov5_cas, OV5_HP_EVT));
>          hp->drc_id.count_indexed.count =
>              cpu_to_be32(drc_id->count_indexed.count);
>          hp->drc_id.count_indexed.index =

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH for-6.0] spapr: Allow memory unplug to always succeed
  2020-12-08  4:30 ` David Gibson
@ 2020-12-08  9:06   ` Greg Kurz
  2020-12-10  3:38     ` David Gibson
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kurz @ 2020-12-08  9:06 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 4694 bytes --]

On Tue, 8 Dec 2020 15:30:04 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Dec 07, 2020 at 02:37:04PM +0100, Greg Kurz wrote:
> > It is currently impossible to hot-unplug a memory device between
> > machine reset and CAS.
> > 
> > (qemu) device_del dimm1
> > Error: Memory hot unplug not supported for this guest
> > 
> > This limitation was introduced in order to provide an explicit
> > error path for older guests that didn't support hot-plug event
> > sources (and thus memory hot-unplug).
> > 
> > The linux kernel has been supporting these since 4.11. All recent
> > enough guests are thus capable of handling the removal of a memory
> > device at all time, including during early boot.
> > 
> > Lift the limitation for the latest machine type. This means that
> > trying to unplug memory from a guest that doesn't support it will
> > likely just do nothing and the memory will only get removed at
> > next reboot. Such older guests can still get the existing behavior
> > by using an older machine type.
> > 
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> 
> Looks like this conflicts with something I've added to for-6.0
> recently.  Can you rebase and resend, please.
> 

I'm not quite sure what for-6.0 you're talking about. Despite
you're recent announcement about moving to gitlab, it seems
that the branch at github is the most up to date.

gitlab:
- HEAD is "xive: Add trace events"
- Date: 26 Nov, 2020

github:
- HEAD is "MAINTAINERS: Add Greg Kurz as co-maintainer for ppc"
- Date: Dec 4, 2020

I've thus based this patch on github. Also, this is based on Connie's
"hw: add compat machines for 6.0" patch...

> > ---
> > Based-on: 20201109173928.1001764-1-cohuck@redhat.com

... maybe I should have made it more clear than just
mentioning the message id ?

I think I'll just wait for Connie's patch to get merged and I'll repost after
you've rebased ppc-for-6.0.

> > ---
> >  include/hw/ppc/spapr.h | 1 +
> >  hw/ppc/spapr.c         | 6 +++++-
> >  hw/ppc/spapr_events.c  | 3 ++-
> >  3 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> > index b7ced9faebf5..7aa630350326 100644
> > --- a/include/hw/ppc/spapr.h
> > +++ b/include/hw/ppc/spapr.h
> > @@ -139,6 +139,7 @@ struct SpaprMachineClass {
> >      hwaddr rma_limit;          /* clamp the RMA to this size */
> >      bool pre_5_1_assoc_refpoints;
> >      bool pre_5_2_numa_associativity;
> > +    bool pre_6_0_memory_unplug;
> >  
> >      bool (*phb_placement)(SpaprMachineState *spapr, uint32_t index,
> >                            uint64_t *buid, hwaddr *pio, 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 7e954bc84bed..f0b26b2af30d 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -4044,7 +4044,8 @@ static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
> >      SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> >  
> >      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> > -        if (spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) {
> > +        if (!smc->pre_6_0_memory_unplug ||
> > +            spapr_ovec_test(sms->ov5_cas, OV5_HP_EVT)) {
> >              spapr_memory_unplug_request(hotplug_dev, dev, errp);
> >          } else {
> >              /* NOTE: this means there is a window after guest reset, prior to
> > @@ -4530,8 +4531,11 @@ DEFINE_SPAPR_MACHINE(6_0, "6.0", true);
> >   */
> >  static void spapr_machine_5_2_class_options(MachineClass *mc)
> >  {
> > +    SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> > +
> >      spapr_machine_6_0_class_options(mc);
> >      compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len);
> > +    smc->pre_6_0_memory_unplug = true;
> >  }
> >  
> >  DEFINE_SPAPR_MACHINE(5_2, "5.2", false);
> > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> > index 1add53547ec3..c30123177b16 100644
> > --- a/hw/ppc/spapr_events.c
> > +++ b/hw/ppc/spapr_events.c
> > @@ -659,7 +659,8 @@ static void spapr_hotplug_req_event(uint8_t hp_id, uint8_t hp_action,
> >          /* we should not be using count_indexed value unless the guest
> >           * supports dedicated hotplug event source
> >           */
> > -        g_assert(spapr_ovec_test(spapr->ov5_cas, OV5_HP_EVT));
> > +        g_assert(!SPAPR_MACHINE_GET_CLASS(spapr)->pre_6_0_memory_unplug ||
> > +                 spapr_ovec_test(spapr->ov5_cas, OV5_HP_EVT));
> >          hp->drc_id.count_indexed.count =
> >              cpu_to_be32(drc_id->count_indexed.count);
> >          hp->drc_id.count_indexed.index =
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH for-6.0] spapr: Allow memory unplug to always succeed
  2020-12-08  9:06   ` Greg Kurz
@ 2020-12-10  3:38     ` David Gibson
  2020-12-10 10:16       ` Greg Kurz
  0 siblings, 1 reply; 5+ messages in thread
From: David Gibson @ 2020-12-10  3:38 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2426 bytes --]

On Tue, Dec 08, 2020 at 10:06:09AM +0100, Greg Kurz wrote:
> On Tue, 8 Dec 2020 15:30:04 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Mon, Dec 07, 2020 at 02:37:04PM +0100, Greg Kurz wrote:
> > > It is currently impossible to hot-unplug a memory device between
> > > machine reset and CAS.
> > > 
> > > (qemu) device_del dimm1
> > > Error: Memory hot unplug not supported for this guest
> > > 
> > > This limitation was introduced in order to provide an explicit
> > > error path for older guests that didn't support hot-plug event
> > > sources (and thus memory hot-unplug).
> > > 
> > > The linux kernel has been supporting these since 4.11. All recent
> > > enough guests are thus capable of handling the removal of a memory
> > > device at all time, including during early boot.
> > > 
> > > Lift the limitation for the latest machine type. This means that
> > > trying to unplug memory from a guest that doesn't support it will
> > > likely just do nothing and the memory will only get removed at
> > > next reboot. Such older guests can still get the existing behavior
> > > by using an older machine type.
> > > 
> > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > 
> > Looks like this conflicts with something I've added to for-6.0
> > recently.  Can you rebase and resend, please.
> > 
> 
> I'm not quite sure what for-6.0 you're talking about. Despite
> you're recent announcement about moving to gitlab, it seems
> that the branch at github is the most up to date.
> 
> gitlab:
> - HEAD is "xive: Add trace events"
> - Date: 26 Nov, 2020
> 
> github:
> - HEAD is "MAINTAINERS: Add Greg Kurz as co-maintainer for ppc"
> - Date: Dec 4, 2020
> 
> I've thus based this patch on github. Also, this is based on Connie's
> "hw: add compat machines for 6.0" patch...
> 
> > > ---
> > > Based-on: 20201109173928.1001764-1-cohuck@redhat.com
> 
> ... maybe I should have made it more clear than just
> mentioning the message id ?
> 
> I think I'll just wait for Connie's patch to get merged and I'll repost after
> you've rebased ppc-for-6.0.

Sorry, I think I forgot to push the latest to either gitlab *or*
github.  I've pushed some new stuff now.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH for-6.0] spapr: Allow memory unplug to always succeed
  2020-12-10  3:38     ` David Gibson
@ 2020-12-10 10:16       ` Greg Kurz
  0 siblings, 0 replies; 5+ messages in thread
From: Greg Kurz @ 2020-12-10 10:16 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2553 bytes --]

On Thu, 10 Dec 2020 14:38:44 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Tue, Dec 08, 2020 at 10:06:09AM +0100, Greg Kurz wrote:
> > On Tue, 8 Dec 2020 15:30:04 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > On Mon, Dec 07, 2020 at 02:37:04PM +0100, Greg Kurz wrote:
> > > > It is currently impossible to hot-unplug a memory device between
> > > > machine reset and CAS.
> > > > 
> > > > (qemu) device_del dimm1
> > > > Error: Memory hot unplug not supported for this guest
> > > > 
> > > > This limitation was introduced in order to provide an explicit
> > > > error path for older guests that didn't support hot-plug event
> > > > sources (and thus memory hot-unplug).
> > > > 
> > > > The linux kernel has been supporting these since 4.11. All recent
> > > > enough guests are thus capable of handling the removal of a memory
> > > > device at all time, including during early boot.
> > > > 
> > > > Lift the limitation for the latest machine type. This means that
> > > > trying to unplug memory from a guest that doesn't support it will
> > > > likely just do nothing and the memory will only get removed at
> > > > next reboot. Such older guests can still get the existing behavior
> > > > by using an older machine type.
> > > > 
> > > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > > 
> > > Looks like this conflicts with something I've added to for-6.0
> > > recently.  Can you rebase and resend, please.
> > > 
> > 
> > I'm not quite sure what for-6.0 you're talking about. Despite
> > you're recent announcement about moving to gitlab, it seems
> > that the branch at github is the most up to date.
> > 
> > gitlab:
> > - HEAD is "xive: Add trace events"
> > - Date: 26 Nov, 2020
> > 
> > github:
> > - HEAD is "MAINTAINERS: Add Greg Kurz as co-maintainer for ppc"
> > - Date: Dec 4, 2020
> > 
> > I've thus based this patch on github. Also, this is based on Connie's
> > "hw: add compat machines for 6.0" patch...
> > 
> > > > ---
> > > > Based-on: 20201109173928.1001764-1-cohuck@redhat.com
> > 
> > ... maybe I should have made it more clear than just
> > mentioning the message id ?
> > 
> > I think I'll just wait for Connie's patch to get merged and I'll repost after
> > you've rebased ppc-for-6.0.
> 
> Sorry, I think I forgot to push the latest to either gitlab *or*
> github.  I've pushed some new stuff now.
> 

Np. I see you now have the 6.0 compat machine types in your tree on gitlab.
This patch should apply flawlessly.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-12-10 10:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-07 13:37 [PATCH for-6.0] spapr: Allow memory unplug to always succeed Greg Kurz
2020-12-08  4:30 ` David Gibson
2020-12-08  9:06   ` Greg Kurz
2020-12-10  3:38     ` David Gibson
2020-12-10 10:16       ` Greg Kurz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.