[PATCH] pnv_lpc: disable reentrancy detection for lpc-hc

[PATCH] pnv_lpc: disable reentrancy detection for lpc-hc

[Alexander Bulekov]

As lpc-hc is designed for re-entrant calls from xscom, mark it
re-entrancy safe.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 hw/ppc/pnv_lpc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index 01f44c19eb..67fd049a7f 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -738,6 +738,8 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
                                 &lpc->opb_master_regs);
     memory_region_init_io(&lpc->lpc_hc_regs, OBJECT(dev), &lpc_hc_ops, lpc,
                           "lpc-hc", LPC_HC_REGS_OPB_SIZE);
+    /* xscom writes to lpc-hc. As such mark lpc-hc re-entrancy safe */
+    lpc->lpc_hc_regs.disable_reentrancy_guard = true;
     memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
                                 &lpc->lpc_hc_regs);
 
-- 
2.39.0

Re: [PATCH] pnv_lpc: disable reentrancy detection for lpc-hc

[Cédric Le Goater]

Hello Alexander

On 5/11/23 10:53, Alexander Bulekov wrote:
> As lpc-hc is designed for re-entrant calls from xscom, mark it
> re-entrancy safe.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>   hw/ppc/pnv_lpc.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index 01f44c19eb..67fd049a7f 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -738,6 +738,8 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>                                   &lpc->opb_master_regs);
>       memory_region_init_io(&lpc->lpc_hc_regs, OBJECT(dev), &lpc_hc_ops, lpc,
>                             "lpc-hc", LPC_HC_REGS_OPB_SIZE);
> +    /* xscom writes to lpc-hc. As such mark lpc-hc re-entrancy safe */
> +    lpc->lpc_hc_regs.disable_reentrancy_guard = true;
>       memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
>                                   &lpc->lpc_hc_regs);
>   

The warning changed :

   qemu-system-ppc64: warning: Blocked re-entrant IO on MemoryRegion: lpc-opb-master at addr: 0x8

I will take a look unless you know exactly what to do.

Thanks,

C.

Re: [PATCH] pnv_lpc: disable reentrancy detection for lpc-hc

[Alexander Bulekov]

On 230511 1104, Cédric Le Goater wrote:
> Hello Alexander
> 
> On 5/11/23 10:53, Alexander Bulekov wrote:
> > As lpc-hc is designed for re-entrant calls from xscom, mark it
> > re-entrancy safe.
> > 
> > Reported-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > ---
> >   hw/ppc/pnv_lpc.c | 2 ++
> >   1 file changed, 2 insertions(+)
> > 
> > diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> > index 01f44c19eb..67fd049a7f 100644
> > --- a/hw/ppc/pnv_lpc.c
> > +++ b/hw/ppc/pnv_lpc.c
> > @@ -738,6 +738,8 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
> >                                   &lpc->opb_master_regs);
> >       memory_region_init_io(&lpc->lpc_hc_regs, OBJECT(dev), &lpc_hc_ops, lpc,
> >                             "lpc-hc", LPC_HC_REGS_OPB_SIZE);
> > +    /* xscom writes to lpc-hc. As such mark lpc-hc re-entrancy safe */
> > +    lpc->lpc_hc_regs.disable_reentrancy_guard = true;
> >       memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
> >                                   &lpc->lpc_hc_regs);
> 
> The warning changed :
> 
>   qemu-system-ppc64: warning: Blocked re-entrant IO on MemoryRegion: lpc-opb-master at addr: 0x8
> 
> I will take a look unless you know exactly what to do.
>

That does not show up for me with "./qemu-system-ppc64 -M powernv8" 
Do I need to boot a kernel to see the message?

I was worried that there might be other re-entrant IO in this device.
Maybe there should be a way to just mark the whole device re-entrancy
safe.

Re: [PATCH] pnv_lpc: disable reentrancy detection for lpc-hc

[Cédric Le Goater]

On 5/11/23 11:15, Alexander Bulekov wrote:
> On 230511 1104, Cédric Le Goater wrote:
>> Hello Alexander
>>
>> On 5/11/23 10:53, Alexander Bulekov wrote:
>>> As lpc-hc is designed for re-entrant calls from xscom, mark it
>>> re-entrancy safe.
>>>
>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
>>> ---
>>>    hw/ppc/pnv_lpc.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
>>> index 01f44c19eb..67fd049a7f 100644
>>> --- a/hw/ppc/pnv_lpc.c
>>> +++ b/hw/ppc/pnv_lpc.c
>>> @@ -738,6 +738,8 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>>>                                    &lpc->opb_master_regs);
>>>        memory_region_init_io(&lpc->lpc_hc_regs, OBJECT(dev), &lpc_hc_ops, lpc,
>>>                              "lpc-hc", LPC_HC_REGS_OPB_SIZE);
>>> +    /* xscom writes to lpc-hc. As such mark lpc-hc re-entrancy safe */
>>> +    lpc->lpc_hc_regs.disable_reentrancy_guard = true;
>>>        memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
>>>                                    &lpc->lpc_hc_regs);
>>
>> The warning changed :
>>
>>    qemu-system-ppc64: warning: Blocked re-entrant IO on MemoryRegion: lpc-opb-master at addr: 0x8
>>
>> I will take a look unless you know exactly what to do.
>>
> 
> That does not show up for me with "./qemu-system-ppc64 -M powernv8"
> Do I need to boot a kernel to see the message?

There are other processors:

powernv10            IBM PowerNV (Non-Virtualized) POWER10
powernv8             IBM PowerNV (Non-Virtualized) POWER8
powernv              IBM PowerNV (Non-Virtualized) POWER9 (alias of powernv9)
powernv9             IBM PowerNV (Non-Virtualized) POWER9

Region lpc->opb_master_regs needs to be tagged also for powernv9 and
powernv10.

Thanks,

C.

> I was worried that there might be other re-entrant IO in this device.
> Maybe there should be a way to just mark the whole device re-entrancy
> safe.

Re: [PATCH] pnv_lpc: disable reentrancy detection for lpc-hc

[Frederic Barrat]

On 11/05/2023 11:15, Alexander Bulekov wrote:
> On 230511 1104, Cédric Le Goater wrote:
>> Hello Alexander
>>
>> On 5/11/23 10:53, Alexander Bulekov wrote:
>>> As lpc-hc is designed for re-entrant calls from xscom, mark it
>>> re-entrancy safe.
>>>
>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
>>> ---
>>>    hw/ppc/pnv_lpc.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
>>> index 01f44c19eb..67fd049a7f 100644
>>> --- a/hw/ppc/pnv_lpc.c
>>> +++ b/hw/ppc/pnv_lpc.c
>>> @@ -738,6 +738,8 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>>>                                    &lpc->opb_master_regs);
>>>        memory_region_init_io(&lpc->lpc_hc_regs, OBJECT(dev), &lpc_hc_ops, lpc,
>>>                              "lpc-hc", LPC_HC_REGS_OPB_SIZE);
>>> +    /* xscom writes to lpc-hc. As such mark lpc-hc re-entrancy safe */
>>> +    lpc->lpc_hc_regs.disable_reentrancy_guard = true;
>>>        memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
>>>                                    &lpc->lpc_hc_regs);
>>
>> The warning changed :
>>
>>    qemu-system-ppc64: warning: Blocked re-entrant IO on MemoryRegion: lpc-opb-master at addr: 0x8
>>
>> I will take a look unless you know exactly what to do.
>>
> 
> That does not show up for me with "./qemu-system-ppc64 -M powernv8"
> Do I need to boot a kernel to see the message?
> 
> I was worried that there might be other re-entrant IO in this device.
> Maybe there should be a way to just mark the whole device re-entrancy
> safe.

Hello,

I was also started hitting it, with machine powernv10. And indeed, 
disabling the check on both lpc_hc_regs and opb_master_regs should be 
all we need (and it's working fine for me).

   Fred

Re: [PATCH] pnv_lpc: disable reentrancy detection for lpc-hc

[Thomas Huth]

On 15/05/2023 14.48, Frederic Barrat wrote:
> 
> 
> On 11/05/2023 11:15, Alexander Bulekov wrote:
>> On 230511 1104, Cédric Le Goater wrote:
>>> Hello Alexander
>>>
>>> On 5/11/23 10:53, Alexander Bulekov wrote:
>>>> As lpc-hc is designed for re-entrant calls from xscom, mark it
>>>> re-entrancy safe.
>>>>
>>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>>> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
>>>> ---
>>>>    hw/ppc/pnv_lpc.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
>>>> index 01f44c19eb..67fd049a7f 100644
>>>> --- a/hw/ppc/pnv_lpc.c
>>>> +++ b/hw/ppc/pnv_lpc.c
>>>> @@ -738,6 +738,8 @@ static void pnv_lpc_realize(DeviceState *dev, Error 
>>>> **errp)
>>>>                                    &lpc->opb_master_regs);
>>>>        memory_region_init_io(&lpc->lpc_hc_regs, OBJECT(dev), 
>>>> &lpc_hc_ops, lpc,
>>>>                              "lpc-hc", LPC_HC_REGS_OPB_SIZE);
>>>> +    /* xscom writes to lpc-hc. As such mark lpc-hc re-entrancy safe */
>>>> +    lpc->lpc_hc_regs.disable_reentrancy_guard = true;
>>>>        memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
>>>>                                    &lpc->lpc_hc_regs);
>>>
>>> The warning changed :
>>>
>>>    qemu-system-ppc64: warning: Blocked re-entrant IO on MemoryRegion: 
>>> lpc-opb-master at addr: 0x8
>>>
>>> I will take a look unless you know exactly what to do.
>>>
>>
>> That does not show up for me with "./qemu-system-ppc64 -M powernv8"
>> Do I need to boot a kernel to see the message?
>>
>> I was worried that there might be other re-entrant IO in this device.
>> Maybe there should be a way to just mark the whole device re-entrancy
>> safe.
> 
> Hello,
> 
> I was also started hitting it, with machine powernv10. And indeed, disabling 
> the check on both lpc_hc_regs and opb_master_regs should be all we need (and 
> it's working fine for me).

Alexander, could you please respin your patch to fix the opb_master_regs, too?

  Thomas

Re: [PATCH] pnv_lpc: disable reentrancy detection for lpc-hc

[Cédric Le Goater]

On 5/26/23 09:06, Thomas Huth wrote:
> On 15/05/2023 14.48, Frederic Barrat wrote:
>>
>>
>> On 11/05/2023 11:15, Alexander Bulekov wrote:
>>> On 230511 1104, Cédric Le Goater wrote:
>>>> Hello Alexander
>>>>
>>>> On 5/11/23 10:53, Alexander Bulekov wrote:
>>>>> As lpc-hc is designed for re-entrant calls from xscom, mark it
>>>>> re-entrancy safe.
>>>>>
>>>>> Reported-by: Thomas Huth <thuth@redhat.com>
>>>>> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
>>>>> ---
>>>>>    hw/ppc/pnv_lpc.c | 2 ++
>>>>>    1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
>>>>> index 01f44c19eb..67fd049a7f 100644
>>>>> --- a/hw/ppc/pnv_lpc.c
>>>>> +++ b/hw/ppc/pnv_lpc.c
>>>>> @@ -738,6 +738,8 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>>>>>                                    &lpc->opb_master_regs);
>>>>>        memory_region_init_io(&lpc->lpc_hc_regs, OBJECT(dev), &lpc_hc_ops, lpc,
>>>>>                              "lpc-hc", LPC_HC_REGS_OPB_SIZE);
>>>>> +    /* xscom writes to lpc-hc. As such mark lpc-hc re-entrancy safe */
>>>>> +    lpc->lpc_hc_regs.disable_reentrancy_guard = true;
>>>>>        memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
>>>>>                                    &lpc->lpc_hc_regs);
>>>>
>>>> The warning changed :
>>>>
>>>>    qemu-system-ppc64: warning: Blocked re-entrant IO on MemoryRegion: lpc-opb-master at addr: 0x8
>>>>
>>>> I will take a look unless you know exactly what to do.
>>>>
>>>
>>> That does not show up for me with "./qemu-system-ppc64 -M powernv8"
>>> Do I need to boot a kernel to see the message?
>>>
>>> I was worried that there might be other re-entrant IO in this device.
>>> Maybe there should be a way to just mark the whole device re-entrancy
>>> safe.
>>
>> Hello,
>>
>> I was also started hitting it, with machine powernv10. And indeed, disabling the check on both lpc_hc_regs and opb_master_regs should be all we need (and it's working fine for me).
> 
> Alexander, could you please respin your patch to fix the opb_master_regs, too?

I will do it.

C.

[PATCH] pnv_lpc: disable reentrancy detection for lpc-hc

[Cédric Le Goater]

From: Alexander Bulekov <alxndr@bu.edu>

As lpc-hc is designed for re-entrant calls from xscom, mark it
re-entrancy safe.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
[clg: mark opb_master_regs as re-entrancy safe also ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv_lpc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index 01f44c19ebba..605d3908617a 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -734,10 +734,13 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
     /* Create MMIO regions for LPC HC and OPB registers */
     memory_region_init_io(&lpc->opb_master_regs, OBJECT(dev), &opb_master_ops,
                           lpc, "lpc-opb-master", LPC_OPB_REGS_OPB_SIZE);
+    lpc->opb_master_regs.disable_reentrancy_guard = true;
     memory_region_add_subregion(&lpc->opb_mr, LPC_OPB_REGS_OPB_ADDR,
                                 &lpc->opb_master_regs);
     memory_region_init_io(&lpc->lpc_hc_regs, OBJECT(dev), &lpc_hc_ops, lpc,
                           "lpc-hc", LPC_HC_REGS_OPB_SIZE);
+    /* xscom writes to lpc-hc. As such mark lpc-hc re-entrancy safe */
+    lpc->lpc_hc_regs.disable_reentrancy_guard = true;
     memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
                                 &lpc->lpc_hc_regs);
 
-- 
2.40.1

Re: [PATCH] pnv_lpc: disable reentrancy detection for lpc-hc

[Frederic Barrat]

On 26/05/2023 09:38, Cédric Le Goater wrote:
> From: Alexander Bulekov <alxndr@bu.edu>
> 
> As lpc-hc is designed for re-entrant calls from xscom, mark it
> re-entrancy safe.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> [clg: mark opb_master_regs as re-entrancy safe also ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---


Reviewed-by: Frederic Barrat <fbarrat@linux.ibm.com>

Thanks,

   Fred


>   hw/ppc/pnv_lpc.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index 01f44c19ebba..605d3908617a 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -734,10 +734,13 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>       /* Create MMIO regions for LPC HC and OPB registers */
>       memory_region_init_io(&lpc->opb_master_regs, OBJECT(dev), &opb_master_ops,
>                             lpc, "lpc-opb-master", LPC_OPB_REGS_OPB_SIZE);
> +    lpc->opb_master_regs.disable_reentrancy_guard = true;
>       memory_region_add_subregion(&lpc->opb_mr, LPC_OPB_REGS_OPB_ADDR,
>                                   &lpc->opb_master_regs);
>       memory_region_init_io(&lpc->lpc_hc_regs, OBJECT(dev), &lpc_hc_ops, lpc,
>                             "lpc-hc", LPC_HC_REGS_OPB_SIZE);
> +    /* xscom writes to lpc-hc. As such mark lpc-hc re-entrancy safe */
> +    lpc->lpc_hc_regs.disable_reentrancy_guard = true;
>       memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
>                                   &lpc->lpc_hc_regs);
>   

Re: [PATCH] pnv_lpc: disable reentrancy detection for lpc-hc

[Thomas Huth]

On 26/05/2023 09.38, Cédric Le Goater wrote:
> From: Alexander Bulekov <alxndr@bu.edu>
> 
> As lpc-hc is designed for re-entrant calls from xscom, mark it
> re-entrancy safe.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> [clg: mark opb_master_regs as re-entrancy safe also ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/ppc/pnv_lpc.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index 01f44c19ebba..605d3908617a 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -734,10 +734,13 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>       /* Create MMIO regions for LPC HC and OPB registers */
>       memory_region_init_io(&lpc->opb_master_regs, OBJECT(dev), &opb_master_ops,
>                             lpc, "lpc-opb-master", LPC_OPB_REGS_OPB_SIZE);
> +    lpc->opb_master_regs.disable_reentrancy_guard = true;
>       memory_region_add_subregion(&lpc->opb_mr, LPC_OPB_REGS_OPB_ADDR,
>                                   &lpc->opb_master_regs);
>       memory_region_init_io(&lpc->lpc_hc_regs, OBJECT(dev), &lpc_hc_ops, lpc,
>                             "lpc-hc", LPC_HC_REGS_OPB_SIZE);
> +    /* xscom writes to lpc-hc. As such mark lpc-hc re-entrancy safe */
> +    lpc->lpc_hc_regs.disable_reentrancy_guard = true;
>       memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
>                                   &lpc->lpc_hc_regs);
>   

Tested-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH] pnv_lpc: disable reentrancy detection for lpc-hc

[Daniel Henrique Barboza]

On 5/26/23 04:38, Cédric Le Goater wrote:
> From: Alexander Bulekov <alxndr@bu.edu>
> 
> As lpc-hc is designed for re-entrant calls from xscom, mark it
> re-entrancy safe.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> [clg: mark opb_master_regs as re-entrancy safe also ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---


Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,


Daniel

>   hw/ppc/pnv_lpc.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index 01f44c19ebba..605d3908617a 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -734,10 +734,13 @@ static void pnv_lpc_realize(DeviceState *dev, Error **errp)
>       /* Create MMIO regions for LPC HC and OPB registers */
>       memory_region_init_io(&lpc->opb_master_regs, OBJECT(dev), &opb_master_ops,
>                             lpc, "lpc-opb-master", LPC_OPB_REGS_OPB_SIZE);
> +    lpc->opb_master_regs.disable_reentrancy_guard = true;
>       memory_region_add_subregion(&lpc->opb_mr, LPC_OPB_REGS_OPB_ADDR,
>                                   &lpc->opb_master_regs);
>       memory_region_init_io(&lpc->lpc_hc_regs, OBJECT(dev), &lpc_hc_ops, lpc,
>                             "lpc-hc", LPC_HC_REGS_OPB_SIZE);
> +    /* xscom writes to lpc-hc. As such mark lpc-hc re-entrancy safe */
> +    lpc->lpc_hc_regs.disable_reentrancy_guard = true;
>       memory_region_add_subregion(&lpc->opb_mr, LPC_HC_REGS_OPB_ADDR,
>                                   &lpc->lpc_hc_regs);
>