[PATCH 4/4] ppc/pnv: Add xscom- prefix to pervasive-control region name

Nicholas Piggin posted 4 patches 11 months, 3 weeks ago
[PATCH 4/4] ppc/pnv: Add xscom- prefix to pervasive-control region name
Posted by Nicholas Piggin 11 months, 3 weeks ago
By convention, xscom regions get a xscom- prefix.

Fixes: 1adf24708bf7 ("hw/ppc: Add pnv nest pervasive common chiplet model")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/pnv_nest_pervasive.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/pnv_nest_pervasive.c b/hw/ppc/pnv_nest_pervasive.c
index 77476753a4..780fa69dde 100644
--- a/hw/ppc/pnv_nest_pervasive.c
+++ b/hw/ppc/pnv_nest_pervasive.c
@@ -177,7 +177,7 @@ static void pnv_nest_pervasive_realize(DeviceState *dev, Error **errp)
     pnv_xscom_region_init(&nest_pervasive->xscom_ctrl_regs_mr,
                           OBJECT(nest_pervasive),
                           &pnv_nest_pervasive_control_xscom_ops,
-                          nest_pervasive, "pervasive-control",
+                          nest_pervasive, "xscom-pervasive-control",
                           PNV10_XSCOM_CHIPLET_CTRL_REGS_SIZE);
 }
 
-- 
2.45.2
Re: [PATCH 4/4] ppc/pnv: Add xscom- prefix to pervasive-control region name
Posted by Philippe Mathieu-Daudé 11 months, 3 weeks ago
Hi,

On 25/11/24 14:20, Nicholas Piggin wrote:
> By convention, xscom regions get a xscom- prefix.
> 
> Fixes: 1adf24708bf7 ("hw/ppc: Add pnv nest pervasive common chiplet model")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   hw/ppc/pnv_nest_pervasive.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/pnv_nest_pervasive.c b/hw/ppc/pnv_nest_pervasive.c
> index 77476753a4..780fa69dde 100644
> --- a/hw/ppc/pnv_nest_pervasive.c
> +++ b/hw/ppc/pnv_nest_pervasive.c
> @@ -177,7 +177,7 @@ static void pnv_nest_pervasive_realize(DeviceState *dev, Error **errp)
>       pnv_xscom_region_init(&nest_pervasive->xscom_ctrl_regs_mr,
>                             OBJECT(nest_pervasive),
>                             &pnv_nest_pervasive_control_xscom_ops,
> -                          nest_pervasive, "pervasive-control",
> +                          nest_pervasive, "xscom-pervasive-control",

Could this break migration stream? Or only RAM regions need to
have a stable name? I don't remember, but try be be cautions with
such cosmetic change just before the release ;)

>                             PNV10_XSCOM_CHIPLET_CTRL_REGS_SIZE);
>   }
>
Re: [PATCH 4/4] ppc/pnv: Add xscom- prefix to pervasive-control region name
Posted by Nicholas Piggin 11 months, 3 weeks ago
On Tue Nov 26, 2024 at 5:28 AM AEST, Philippe Mathieu-Daudé wrote:
> Hi,
>
> On 25/11/24 14:20, Nicholas Piggin wrote:
> > By convention, xscom regions get a xscom- prefix.
> > 
> > Fixes: 1adf24708bf7 ("hw/ppc: Add pnv nest pervasive common chiplet model")
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   hw/ppc/pnv_nest_pervasive.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/ppc/pnv_nest_pervasive.c b/hw/ppc/pnv_nest_pervasive.c
> > index 77476753a4..780fa69dde 100644
> > --- a/hw/ppc/pnv_nest_pervasive.c
> > +++ b/hw/ppc/pnv_nest_pervasive.c
> > @@ -177,7 +177,7 @@ static void pnv_nest_pervasive_realize(DeviceState *dev, Error **errp)
> >       pnv_xscom_region_init(&nest_pervasive->xscom_ctrl_regs_mr,
> >                             OBJECT(nest_pervasive),
> >                             &pnv_nest_pervasive_control_xscom_ops,
> > -                          nest_pervasive, "pervasive-control",
> > +                          nest_pervasive, "xscom-pervasive-control",
>
> Could this break migration stream? Or only RAM regions need to
> have a stable name? I don't remember, but try be be cautions with
> such cosmetic change just before the release ;)

Hey Phil,

Thanks for always somehow being across everything :)

For the powernv machine we are okay since we don't support migration
at all. It's on the wishlist, but at the moment we have lots of
hardware models that don't provide vmstate. So I think we are okay.

Thanks,
Nick
Re: [PATCH 4/4] ppc/pnv: Add xscom- prefix to pervasive-control region name
Posted by Philippe Mathieu-Daudé 11 months, 3 weeks ago
On 26/11/24 04:54, Nicholas Piggin wrote:
> On Tue Nov 26, 2024 at 5:28 AM AEST, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> On 25/11/24 14:20, Nicholas Piggin wrote:
>>> By convention, xscom regions get a xscom- prefix.
>>>
>>> Fixes: 1adf24708bf7 ("hw/ppc: Add pnv nest pervasive common chiplet model")
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
>>>    hw/ppc/pnv_nest_pervasive.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ppc/pnv_nest_pervasive.c b/hw/ppc/pnv_nest_pervasive.c
>>> index 77476753a4..780fa69dde 100644
>>> --- a/hw/ppc/pnv_nest_pervasive.c
>>> +++ b/hw/ppc/pnv_nest_pervasive.c
>>> @@ -177,7 +177,7 @@ static void pnv_nest_pervasive_realize(DeviceState *dev, Error **errp)
>>>        pnv_xscom_region_init(&nest_pervasive->xscom_ctrl_regs_mr,
>>>                              OBJECT(nest_pervasive),
>>>                              &pnv_nest_pervasive_control_xscom_ops,
>>> -                          nest_pervasive, "pervasive-control",
>>> +                          nest_pervasive, "xscom-pervasive-control",
>>
>> Could this break migration stream? Or only RAM regions need to
>> have a stable name? I don't remember, but try be be cautions with
>> such cosmetic change just before the release ;)
> 
> Hey Phil,
> 
> Thanks for always somehow being across everything :)

;)

Answering myself, I/O regions are migrated within the device state,
per field, via DeviceClass::vmsd structure.

IIRC RAM regions are the ones tied to their name, which is built
using the object owner, which is why we can't change migrated RAM
MR owners.

> For the powernv machine we are okay since we don't support migration
> at all. It's on the wishlist, but at the moment we have lots of
> hardware models that don't provide vmstate. So I think we are okay.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Re: [PATCH 4/4] ppc/pnv: Add xscom- prefix to pervasive-control region name
Posted by Peter Xu 11 months, 3 weeks ago
On Tue, Nov 26, 2024 at 06:19:52AM +0100, Philippe Mathieu-Daudé wrote:
> On 26/11/24 04:54, Nicholas Piggin wrote:
> > On Tue Nov 26, 2024 at 5:28 AM AEST, Philippe Mathieu-Daudé wrote:
> > > Hi,
> > > 
> > > On 25/11/24 14:20, Nicholas Piggin wrote:
> > > > By convention, xscom regions get a xscom- prefix.
> > > > 
> > > > Fixes: 1adf24708bf7 ("hw/ppc: Add pnv nest pervasive common chiplet model")
> > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > > > ---
> > > >    hw/ppc/pnv_nest_pervasive.c | 2 +-
> > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/hw/ppc/pnv_nest_pervasive.c b/hw/ppc/pnv_nest_pervasive.c
> > > > index 77476753a4..780fa69dde 100644
> > > > --- a/hw/ppc/pnv_nest_pervasive.c
> > > > +++ b/hw/ppc/pnv_nest_pervasive.c
> > > > @@ -177,7 +177,7 @@ static void pnv_nest_pervasive_realize(DeviceState *dev, Error **errp)
> > > >        pnv_xscom_region_init(&nest_pervasive->xscom_ctrl_regs_mr,
> > > >                              OBJECT(nest_pervasive),
> > > >                              &pnv_nest_pervasive_control_xscom_ops,
> > > > -                          nest_pervasive, "pervasive-control",
> > > > +                          nest_pervasive, "xscom-pervasive-control",
> > > 
> > > Could this break migration stream? Or only RAM regions need to
> > > have a stable name? I don't remember, but try be be cautions with
> > > such cosmetic change just before the release ;)
> > 
> > Hey Phil,
> > 
> > Thanks for always somehow being across everything :)
> 
> ;)
> 
> Answering myself, I/O regions are migrated within the device state,
> per field, via DeviceClass::vmsd structure.

IIUC by default we don't migrate IO regions at all, unless we register
something into the VMSD explicitly.

> 
> IIRC RAM regions are the ones tied to their name, which is built
> using the object owner, which is why we can't change migrated RAM
> MR owners.

True.

Thanks,

-- 
Peter Xu


Re: [PATCH 4/4] ppc/pnv: Add xscom- prefix to pervasive-control region name
Posted by Miles Glenn 11 months, 3 weeks ago
Reviewed-by: Glenn Miles <milesg@linux.ibm.com>

On Mon, 2024-11-25 at 23:20 +1000, Nicholas Piggin wrote:
> By convention, xscom regions get a xscom- prefix.
> 
> Fixes: 1adf24708bf7 ("hw/ppc: Add pnv nest pervasive common chiplet
> model")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  hw/ppc/pnv_nest_pervasive.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/pnv_nest_pervasive.c
> b/hw/ppc/pnv_nest_pervasive.c
> index 77476753a4..780fa69dde 100644
> --- a/hw/ppc/pnv_nest_pervasive.c
> +++ b/hw/ppc/pnv_nest_pervasive.c
> @@ -177,7 +177,7 @@ static void
> pnv_nest_pervasive_realize(DeviceState *dev, Error **errp)
>      pnv_xscom_region_init(&nest_pervasive->xscom_ctrl_regs_mr,
>                            OBJECT(nest_pervasive),
>                            &pnv_nest_pervasive_control_xscom_ops,
> -                          nest_pervasive, "pervasive-control",
> +                          nest_pervasive, "xscom-pervasive-control",
>                            PNV10_XSCOM_CHIPLET_CTRL_REGS_SIZE);
>  }
>