All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Cédric Le Goater" <clg@kaod.org>
To: Daniel Henrique Barboza <danielhb413@gmail.com>, <qemu-devel@nongnu.org>
Cc: qemu-ppc@nongnu.org, david@gibson.dropbear.id.au
Subject: Re: [PATCH v2 6/8] ppc/pnv: make PECs create and realize PHB4s
Date: Mon, 17 Jan 2022 11:55:55 +0100	[thread overview]
Message-ID: <365e2073-8664-0d01-15db-5aaf9fad2999@kaod.org> (raw)
In-Reply-To: <20220114180719.52117-7-danielhb413@gmail.com>

On 1/14/22 19:07, Daniel Henrique Barboza wrote:
> This patch changes the design of the PEC device to create and realize PHB4s
> instead of PecStacks. After all the recent changes, PHB4s now contain all
> the information needed for their proper functioning, not relying on PecStack
> in any capacity.
> 
> All changes are being made in a single patch to avoid renaming parts of
> the PecState and leaving the code in a strange way. E.g. rename
> PecClass->num_stacks to num_phbs, which would then read a
> pnv_pec_num_stacks[] array. To avoid mixing the old and new design more
> than necessary it's clearer to do these changes in a single step.
> 
> The name changes made are:
> 
> - in PnvPhb4PecState:
>    * rename 'num_stacks' to 'num_phbs'
>    * remove the pec->stacks[] array. Current code relies on the
> pec->stacks[] obj acting as a simple container, without ever accessing
> pec->stacks[] for any other purpose. Instead of converting this into a
> pec->phbs[] array, remove it
> 
> - in PnvPhb4PecClass, rename *num_stacks to *num_phbs;
> 
> - pnv_pec_num_stacks[] is renamed to pnv_pec_num_phbs[].
> 
> The logical changes:
> 
> - pnv_pec_default_phb_realize():
>    * init and set the properties of the PnvPHB4 qdev
>    * do not use stack->phb anymore;
> 
> - pnv_pec_realize():
>    * use the new default_phb_realize() to init/realize each PHB if
> running with defaults;
> 
> - pnv_pec_instance_init(): removed since we're creating the PHBs during
> pec_realize();
> 
> - pnv_phb4_get_stack():
>    * renamed to pnv_phb4_get_pec() and returns a PnvPhb4PecState*;
> 
> - pnv_phb4_realize(): use 'phb->pec' instead of 'stack'.
> 
> This design change shouldn't caused any behavioral change in the runtime
> of the machine.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   hw/pci-host/pnv_phb4.c         | 26 ++++++--------
>   hw/pci-host/pnv_phb4_pec.c     | 66 ++++++++++------------------------
>   include/hw/pci-host/pnv_phb4.h |  8 ++---
>   3 files changed, 31 insertions(+), 69 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 2efd34518e..3dc3c70cb2 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -884,7 +884,7 @@ static int pnv_phb4_get_phb_stack_no(PnvPHB4 *phb)
>       int stack_no = phb->phb_id;
>   
>       while (index--) {
> -        stack_no -= pecc->num_stacks[index];
> +        stack_no -= pecc->num_phbs[index];
>       }
>   
>       return stack_no;
> @@ -1383,7 +1383,7 @@ int pnv_phb4_pec_get_phb_id(PnvPhb4PecState *pec, int stack_index)
>       int offset = 0;
>   
>       while (index--) {
> -        offset += pecc->num_stacks[index];
> +        offset += pecc->num_phbs[index];
>       }
>   
>       return offset + stack_index;
> @@ -1534,8 +1534,8 @@ static void pnv_phb4_instance_init(Object *obj)
>       object_initialize_child(obj, "source", &phb->xsrc, TYPE_XIVE_SOURCE);
>   }
>   
> -static PnvPhb4PecStack *pnv_phb4_get_stack(PnvChip *chip, PnvPHB4 *phb,
> -                                           Error **errp)
> +static PnvPhb4PecState *pnv_phb4_get_pec(PnvChip *chip, PnvPHB4 *phb,
> +                                         Error **errp)
>   {
>       Pnv9Chip *chip9 = PNV9_CHIP(chip);
>       int chip_id = phb->chip_id;
> @@ -1544,14 +1544,14 @@ static PnvPhb4PecStack *pnv_phb4_get_stack(PnvChip *chip, PnvPHB4 *phb,
>   
>       for (i = 0; i < chip->num_pecs; i++) {
>           /*
> -         * For each PEC, check the amount of stacks it supports
> -         * and see if the given phb4 index matches a stack.
> +         * For each PEC, check the amount of phbs it supports
> +         * and see if the given phb4 index matches an index.
>            */
>           PnvPhb4PecState *pec = &chip9->pecs[i];
>   
> -        for (j = 0; j < pec->num_stacks; j++) {
> +        for (j = 0; j < pec->num_phbs; j++) {
>               if (index == pnv_phb4_pec_get_phb_id(pec, j)) {
> -                return &pec->stacks[j];
> +                return pec;
>               }
>           }
>       }
> @@ -1576,7 +1576,6 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>       if (!phb->pec) {
>           PnvMachineState *pnv = PNV_MACHINE(qdev_get_machine());
>           PnvChip *chip = pnv_get_chip(pnv, phb->chip_id);
> -        PnvPhb4PecStack *stack;
>           PnvPhb4PecClass *pecc;
>           BusState *s;
>   
> @@ -1585,18 +1584,13 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>               return;
>           }
>   
> -        stack = pnv_phb4_get_stack(chip, phb, &local_err);
> +        phb->pec = pnv_phb4_get_pec(chip, phb, &local_err);
>           if (local_err) {
>               error_propagate(errp, local_err);
>               return;
>           }
>   
> -        /*
> -         * All other phb properties but 'pec' ad 'version' are
> -         * already set.
> -         */
> -        object_property_set_link(OBJECT(phb), "pec", OBJECT(stack->pec),
> -                                 &error_abort);
> +        /* All other phb properties are already set */
>           pecc = PNV_PHB4_PEC_GET_CLASS(phb->pec);
>           object_property_set_int(OBJECT(phb), "version", pecc->version,
>                                   &error_fatal);
> diff --git a/hw/pci-host/pnv_phb4_pec.c b/hw/pci-host/pnv_phb4_pec.c
> index d6405d6ca3..852816b9f8 100644
> --- a/hw/pci-host/pnv_phb4_pec.c
> +++ b/hw/pci-host/pnv_phb4_pec.c
> @@ -112,41 +112,28 @@ static const MemoryRegionOps pnv_pec_pci_xscom_ops = {
>       .endianness = DEVICE_BIG_ENDIAN,
>   };
>   
> -static void pnv_pec_default_phb_realize(PnvPhb4PecStack *stack,
> +static void pnv_pec_default_phb_realize(PnvPhb4PecState *pec,
>                                           int stack_no,
>                                           Error **errp)
>   {
> -    PnvPhb4PecState *pec = stack->pec;
> +    PnvPHB4 *phb = PNV_PHB4(qdev_new(TYPE_PNV_PHB4));
>       PnvPhb4PecClass *pecc = PNV_PHB4_PEC_GET_CLASS(pec);
>       int phb_id = pnv_phb4_pec_get_phb_id(pec, stack_no);
>   
> -    stack->phb = PNV_PHB4(qdev_new(TYPE_PNV_PHB4));
> -
> -    object_property_set_link(OBJECT(stack->phb), "pec", OBJECT(pec),
> +    object_property_set_link(OBJECT(phb), "pec", OBJECT(pec),
>                                &error_abort);
> -    object_property_set_int(OBJECT(stack->phb), "chip-id", pec->chip_id,
> +    object_property_set_int(OBJECT(phb), "chip-id", pec->chip_id,
>                               &error_fatal);
> -    object_property_set_int(OBJECT(stack->phb), "index", phb_id,
> +    object_property_set_int(OBJECT(phb), "index", phb_id,
>                               &error_fatal);
> -    object_property_set_int(OBJECT(stack->phb), "version", pecc->version,
> +    object_property_set_int(OBJECT(phb), "version", pecc->version,
>                               &error_fatal);
>   
> -    if (!sysbus_realize(SYS_BUS_DEVICE(stack->phb), errp)) {
> +    if (!sysbus_realize(SYS_BUS_DEVICE(phb), errp)) {
>           return;
>       }
>   }
>   
> -static void pnv_pec_instance_init(Object *obj)
> -{
> -    PnvPhb4PecState *pec = PNV_PHB4_PEC(obj);
> -    int i;
> -
> -    for (i = 0; i < PHB4_PEC_MAX_STACKS; i++) {
> -        object_initialize_child(obj, "stack[*]", &pec->stacks[i],
> -                                TYPE_PNV_PHB4_PEC_STACK);
> -    }
> -}
> -
>   static void pnv_pec_realize(DeviceState *dev, Error **errp)
>   {
>       PnvPhb4PecState *pec = PNV_PHB4_PEC(dev);
> @@ -159,29 +146,13 @@ static void pnv_pec_realize(DeviceState *dev, Error **errp)
>           return;
>       }
>   
> -    pec->num_stacks = pecc->num_stacks[pec->index];
> -
> -    /* Create stacks */
> -    for (i = 0; i < pec->num_stacks; i++) {
> -        PnvPhb4PecStack *stack = &pec->stacks[i];
> -        Object *stk_obj = OBJECT(stack);
> -
> -        object_property_set_link(stk_obj, "pec", OBJECT(pec), &error_abort);
> +    pec->num_phbs = pecc->num_phbs[pec->index];
>   
> -        if (defaults_enabled()) {
> -            pnv_pec_default_phb_realize(stack, i, errp);
> +    /* Create PHBs if running with defaults */
> +    if (defaults_enabled()) {
> +        for (i = 0; i < pec->num_phbs; i++) {
> +            pnv_pec_default_phb_realize(pec, i, errp);
>           }
> -
> -        /*
> -         * qdev gets angry if we don't realize 'stack' here, even
> -         * if stk_realize() is now empty.
> -         */
> -        if (!qdev_realize(DEVICE(stk_obj), NULL, errp)) {
> -            return;
> -        }
> -    }
> -    for (; i < PHB4_PEC_MAX_STACKS; i++) {
> -        object_unparent(OBJECT(&pec->stacks[i]));
>       }
>   
>       /* Initialize the XSCOM regions for the PEC registers */
> @@ -227,7 +198,7 @@ static int pnv_pec_dt_xscom(PnvXScomInterface *dev, void *fdt,
>       _FDT((fdt_setprop(fdt, offset, "compatible", pecc->compat,
>                         pecc->compat_size)));
>   
> -    for (i = 0; i < pec->num_stacks; i++) {
> +    for (i = 0; i < pec->num_phbs; i++) {
>           int phb_id = pnv_phb4_pec_get_phb_id(pec, i);
>           int stk_offset;
>   
> @@ -263,11 +234,11 @@ static uint32_t pnv_pec_xscom_nest_base(PnvPhb4PecState *pec)
>   }
>   
>   /*
> - * PEC0 -> 1 stack
> - * PEC1 -> 2 stacks
> - * PEC2 -> 3 stacks
> + * PEC0 -> 1 phb
> + * PEC1 -> 2 phb
> + * PEC2 -> 3 phbs
>    */
> -static const uint32_t pnv_pec_num_stacks[] = { 1, 2, 3 };
> +static const uint32_t pnv_pec_num_phbs[] = { 1, 2, 3 };
>   
>   static void pnv_pec_class_init(ObjectClass *klass, void *data)
>   {
> @@ -292,14 +263,13 @@ static void pnv_pec_class_init(ObjectClass *klass, void *data)
>       pecc->stk_compat = stk_compat;
>       pecc->stk_compat_size = sizeof(stk_compat);
>       pecc->version = PNV_PHB4_VERSION;
> -    pecc->num_stacks = pnv_pec_num_stacks;
> +    pecc->num_phbs = pnv_pec_num_phbs;
>   }
>   
>   static const TypeInfo pnv_pec_type_info = {
>       .name          = TYPE_PNV_PHB4_PEC,
>       .parent        = TYPE_DEVICE,
>       .instance_size = sizeof(PnvPhb4PecState),
> -    .instance_init = pnv_pec_instance_init,
>       .class_init    = pnv_pec_class_init,
>       .class_size    = sizeof(PnvPhb4PecClass),
>       .interfaces    = (InterfaceInfo[]) {
> diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
> index 2be56b7afd..e750165e77 100644
> --- a/include/hw/pci-host/pnv_phb4.h
> +++ b/include/hw/pci-host/pnv_phb4.h
> @@ -200,10 +200,8 @@ struct PnvPhb4PecState {
>       uint64_t pci_regs[PHB4_PEC_PCI_REGS_COUNT];
>       MemoryRegion pci_regs_mr;
>   
> -    /* Stacks */
> -    #define PHB4_PEC_MAX_STACKS     3
> -    uint32_t num_stacks;
> -    PnvPhb4PecStack stacks[PHB4_PEC_MAX_STACKS];
> +    /* PHBs */
> +    uint32_t num_phbs;
>   
>       PnvChip *chip;
>   };
> @@ -221,7 +219,7 @@ struct PnvPhb4PecClass {
>       const char *stk_compat;
>       int stk_compat_size;
>       uint64_t version;
> -    const uint32_t *num_stacks;
> +    const uint32_t *num_phbs;
>   };
>   
>   #endif /* PCI_HOST_PNV_PHB4_H */
> 



  reply	other threads:[~2022-01-17 11:06 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-14 18:07 [PATCH v2 0/8] remove PnvPhb4PecStack from Powernv9 Daniel Henrique Barboza
2022-01-14 18:07 ` [PATCH v2 1/8] ppc/pnv: introduce PnvPHB4 'pec' property Daniel Henrique Barboza
2022-01-14 18:07 ` [PATCH v2 2/8] ppc/pnv: reduce stack->stack_no usage Daniel Henrique Barboza
2022-01-17 10:52   ` Cédric Le Goater
2022-01-14 18:07 ` [PATCH v2 3/8] ppc/pnv: remove stack pointer from PnvPHB4 Daniel Henrique Barboza
2022-01-14 18:07 ` [PATCH v2 4/8] ppc/pnv: move default_phb_realize() to pec_realize() Daniel Henrique Barboza
2022-01-14 18:07 ` [PATCH v2 5/8] ppc/pnv: remove PnvPhb4PecStack::stack_no Daniel Henrique Barboza
2022-01-17 10:53   ` Cédric Le Goater
2022-01-14 18:07 ` [PATCH v2 6/8] ppc/pnv: make PECs create and realize PHB4s Daniel Henrique Barboza
2022-01-17 10:55   ` Cédric Le Goater [this message]
2022-01-14 18:07 ` [PATCH v2 7/8] ppc/pnv: remove PnvPhb4PecStack object Daniel Henrique Barboza
2022-01-14 18:07 ` [PATCH v2 8/8] ppc/pnv: rename pnv_pec_stk_update_map() Daniel Henrique Barboza
2022-01-18 12:03 ` [PATCH v2 0/8] remove PnvPhb4PecStack from Powernv9 Cédric Le Goater

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=365e2073-8664-0d01-15db-5aaf9fad2999@kaod.org \
    --to=clg@kaod.org \
    --cc=danielhb413@gmail.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.