[PATCH 0/4] target/ppc: TCG SMT support for spapr

Nicholas Piggin posted 4 patches 11 months ago
Failed in applying to current master (apply log)
hw/ppc/ppc.c             |  6 ++++
hw/ppc/spapr.c           | 16 +++++++---
hw/ppc/spapr_caps.c      | 14 ++++++++
hw/ppc/spapr_cpu_core.c  |  7 ++--
include/hw/ppc/ppc.h     |  1 +
target/ppc/cpu.h         |  9 ++++++
target/ppc/cpu_init.c    |  5 +++
target/ppc/excp_helper.c | 30 ++++++++++++++---
target/ppc/gdbstub.c     |  2 +-
target/ppc/helper.h      |  2 ++
target/ppc/misc_helper.c | 69 ++++++++++++++++++++++++++++++++++++----
target/ppc/translate.c   | 46 ++++++++++++++++++++++++++-
12 files changed, 188 insertions(+), 19 deletions(-)
[PATCH 0/4] target/ppc: TCG SMT support for spapr
Posted by Nicholas Piggin 11 months ago
Previous RFC here

https://lists.gnu.org/archive/html/qemu-ppc/2023-05/msg00453.html

This series drops patch 1 from the previous, which is more of
a standalone bugfix.

Also accounted for Cedric's comments, except a nicer way to
set cpu_index vs PIR/TIR SPRs, which is not quite trivial.

This limits support for SMT to POWER8 and newer. It is also
incompatible with nested-HV so that is checked for too.

Iterating CPUs to find siblings for now I kept because similar
loops exist in a few places, and it is not conceptually
difficult for SMT, just fiddly code to improve. For now it
should not be much performane concern.

I removed hypervisor msgsnd support from patch 3, which is not
required for spapr and added significantly to the patch.

For now nobody has objected to the way shared SPR access is
handled (serialised with TCG atomics support) so we'll keep
going with it.

Thanks,
Nick

Nicholas Piggin (4):
  target/ppc: Add initial flags and helpers for SMT support
  target/ppc: Add support for SMT CTRL register
  target/ppc: Add msgsndp and DPDES SMT support
  spapr: Allow up to 8 threads SMT on POWER8 and newer

 hw/ppc/ppc.c             |  6 ++++
 hw/ppc/spapr.c           | 16 +++++++---
 hw/ppc/spapr_caps.c      | 14 ++++++++
 hw/ppc/spapr_cpu_core.c  |  7 ++--
 include/hw/ppc/ppc.h     |  1 +
 target/ppc/cpu.h         |  9 ++++++
 target/ppc/cpu_init.c    |  5 +++
 target/ppc/excp_helper.c | 30 ++++++++++++++---
 target/ppc/gdbstub.c     |  2 +-
 target/ppc/helper.h      |  2 ++
 target/ppc/misc_helper.c | 69 ++++++++++++++++++++++++++++++++++++----
 target/ppc/translate.c   | 46 ++++++++++++++++++++++++++-
 12 files changed, 188 insertions(+), 19 deletions(-)

-- 
2.40.1
Re: [PATCH 0/4] target/ppc: TCG SMT support for spapr
Posted by Cédric Le Goater 11 months ago
On 6/5/23 13:23, Nicholas Piggin wrote:
> Previous RFC here
> 
> https://lists.gnu.org/archive/html/qemu-ppc/2023-05/msg00453.html
> 
> This series drops patch 1 from the previous, which is more of
> a standalone bugfix.
> 
> Also accounted for Cedric's comments, except a nicer way to
> set cpu_index vs PIR/TIR SPRs, which is not quite trivial.
> 
> This limits support for SMT to POWER8 and newer. It is also
> incompatible with nested-HV so that is checked for too.
> 
> Iterating CPUs to find siblings for now I kept because similar
> loops exist in a few places, and it is not conceptually
> difficult for SMT, just fiddly code to improve. For now it
> should not be much performane concern.
> 
> I removed hypervisor msgsnd support from patch 3, which is not
> required for spapr and added significantly to the patch.
> 
> For now nobody has objected to the way shared SPR access is
> handled (serialised with TCG atomics support) so we'll keep
> going with it.

Cc:ing more people for possible feedback.

Thanks,

C.
Re: [PATCH 0/4] target/ppc: TCG SMT support for spapr
Posted by Nicholas Piggin 10 months, 2 weeks ago
On Wed Jun 7, 2023 at 12:09 AM AEST, Cédric Le Goater wrote:
> On 6/5/23 13:23, Nicholas Piggin wrote:
> > Previous RFC here
> > 
> > https://lists.gnu.org/archive/html/qemu-ppc/2023-05/msg00453.html
> > 
> > This series drops patch 1 from the previous, which is more of
> > a standalone bugfix.
> > 
> > Also accounted for Cedric's comments, except a nicer way to
> > set cpu_index vs PIR/TIR SPRs, which is not quite trivial.
> > 
> > This limits support for SMT to POWER8 and newer. It is also
> > incompatible with nested-HV so that is checked for too.
> > 
> > Iterating CPUs to find siblings for now I kept because similar
> > loops exist in a few places, and it is not conceptually
> > difficult for SMT, just fiddly code to improve. For now it
> > should not be much performane concern.
> > 
> > I removed hypervisor msgsnd support from patch 3, which is not
> > required for spapr and added significantly to the patch.
> > 
> > For now nobody has objected to the way shared SPR access is
> > handled (serialised with TCG atomics support) so we'll keep
> > going with it.
>
> Cc:ing more people for possible feedback.

Not much feedback so I'll plan to go with this.

A more performant implementation might try to synchronize
threads at the register level rather than serialize everything,
but SMT shared registers are not too performance critical so
this should do for now.

Thanks,
Nick
Re: [PATCH 0/4] target/ppc: TCG SMT support for spapr
Posted by Cédric Le Goater 10 months, 2 weeks ago
On 6/20/23 12:12, Nicholas Piggin wrote:
> On Wed Jun 7, 2023 at 12:09 AM AEST, Cédric Le Goater wrote:
>> On 6/5/23 13:23, Nicholas Piggin wrote:
>>> Previous RFC here
>>>
>>> https://lists.gnu.org/archive/html/qemu-ppc/2023-05/msg00453.html
>>>
>>> This series drops patch 1 from the previous, which is more of
>>> a standalone bugfix.
>>>
>>> Also accounted for Cedric's comments, except a nicer way to
>>> set cpu_index vs PIR/TIR SPRs, which is not quite trivial.
>>>
>>> This limits support for SMT to POWER8 and newer. It is also
>>> incompatible with nested-HV so that is checked for too.
>>>
>>> Iterating CPUs to find siblings for now I kept because similar
>>> loops exist in a few places, and it is not conceptually
>>> difficult for SMT, just fiddly code to improve. For now it
>>> should not be much performane concern.
>>>
>>> I removed hypervisor msgsnd support from patch 3, which is not
>>> required for spapr and added significantly to the patch.
>>>
>>> For now nobody has objected to the way shared SPR access is
>>> handled (serialised with TCG atomics support) so we'll keep
>>> going with it.
>>
>> Cc:ing more people for possible feedback.
> 
> Not much feedback so I'll plan to go with this.
> 
> A more performant implementation might try to synchronize
> threads at the register level rather than serialize everything,
> but SMT shared registers are not too performance critical so
> this should do for now.

yes. Could you please rebase this series on upstream ?

It would be good to add tests for SMT. May be we could extend :

   tests/avocado/ppc_pseries.py

with a couple of extra QEMU configs adding 'threads=' (if possible) and
check :

   "CPU maps initialized for Y threads per core"

and

   "smp: Brought up 1 node, X*Y CPUs"

?

Thanks,

C.

Re: [PATCH 0/4] target/ppc: TCG SMT support for spapr
Posted by Nicholas Piggin 10 months, 2 weeks ago
On Tue Jun 20, 2023 at 8:27 PM AEST, Cédric Le Goater wrote:
> On 6/20/23 12:12, Nicholas Piggin wrote:
> > On Wed Jun 7, 2023 at 12:09 AM AEST, Cédric Le Goater wrote:
> >> On 6/5/23 13:23, Nicholas Piggin wrote:
> >>> Previous RFC here
> >>>
> >>> https://lists.gnu.org/archive/html/qemu-ppc/2023-05/msg00453.html
> >>>
> >>> This series drops patch 1 from the previous, which is more of
> >>> a standalone bugfix.
> >>>
> >>> Also accounted for Cedric's comments, except a nicer way to
> >>> set cpu_index vs PIR/TIR SPRs, which is not quite trivial.
> >>>
> >>> This limits support for SMT to POWER8 and newer. It is also
> >>> incompatible with nested-HV so that is checked for too.
> >>>
> >>> Iterating CPUs to find siblings for now I kept because similar
> >>> loops exist in a few places, and it is not conceptually
> >>> difficult for SMT, just fiddly code to improve. For now it
> >>> should not be much performane concern.
> >>>
> >>> I removed hypervisor msgsnd support from patch 3, which is not
> >>> required for spapr and added significantly to the patch.
> >>>
> >>> For now nobody has objected to the way shared SPR access is
> >>> handled (serialised with TCG atomics support) so we'll keep
> >>> going with it.
> >>
> >> Cc:ing more people for possible feedback.
> > 
> > Not much feedback so I'll plan to go with this.
> > 
> > A more performant implementation might try to synchronize
> > threads at the register level rather than serialize everything,
> > but SMT shared registers are not too performance critical so
> > this should do for now.
>
> yes. Could you please rebase this series on upstream ?

Oh yeah I should have said, I will rebase and resend.

> It would be good to add tests for SMT. May be we could extend :
>
>    tests/avocado/ppc_pseries.py
>
> with a couple of extra QEMU configs adding 'threads=' (if possible) and
> check :
>
>    "CPU maps initialized for Y threads per core"
>
> and
>
>    "smp: Brought up 1 node, X*Y CPUs"
>
> ?

Yeah that could be a good idea, I'll try it.

Thanks,
Nick
Re: [PATCH 0/4] target/ppc: TCG SMT support for spapr
Posted by Cédric Le Goater 10 months, 2 weeks ago
On 6/20/23 12:45, Nicholas Piggin wrote:
> On Tue Jun 20, 2023 at 8:27 PM AEST, Cédric Le Goater wrote:
>> On 6/20/23 12:12, Nicholas Piggin wrote:
>>> On Wed Jun 7, 2023 at 12:09 AM AEST, Cédric Le Goater wrote:
>>>> On 6/5/23 13:23, Nicholas Piggin wrote:
>>>>> Previous RFC here
>>>>>
>>>>> https://lists.gnu.org/archive/html/qemu-ppc/2023-05/msg00453.html
>>>>>
>>>>> This series drops patch 1 from the previous, which is more of
>>>>> a standalone bugfix.
>>>>>
>>>>> Also accounted for Cedric's comments, except a nicer way to
>>>>> set cpu_index vs PIR/TIR SPRs, which is not quite trivial.
>>>>>
>>>>> This limits support for SMT to POWER8 and newer. It is also
>>>>> incompatible with nested-HV so that is checked for too.
>>>>>
>>>>> Iterating CPUs to find siblings for now I kept because similar
>>>>> loops exist in a few places, and it is not conceptually
>>>>> difficult for SMT, just fiddly code to improve. For now it
>>>>> should not be much performane concern.
>>>>>
>>>>> I removed hypervisor msgsnd support from patch 3, which is not
>>>>> required for spapr and added significantly to the patch.
>>>>>
>>>>> For now nobody has objected to the way shared SPR access is
>>>>> handled (serialised with TCG atomics support) so we'll keep
>>>>> going with it.
>>>>
>>>> Cc:ing more people for possible feedback.
>>>
>>> Not much feedback so I'll plan to go with this.
>>>
>>> A more performant implementation might try to synchronize
>>> threads at the register level rather than serialize everything,
>>> but SMT shared registers are not too performance critical so
>>> this should do for now.
>>
>> yes. Could you please rebase this series on upstream ?
> 
> Oh yeah I should have said, I will rebase and resend.

Here is a tree to check the rebase on :

   https://github.com/legoater/qemu/commits/ppc-next

>> It would be good to add tests for SMT. May be we could extend :
>>
>>     tests/avocado/ppc_pseries.py
>>
>> with a couple of extra QEMU configs adding 'threads=' (if possible) and
>> check :
>>
>>     "CPU maps initialized for Y threads per core"
>>
>> and
>>
>>     "smp: Brought up 1 node, X*Y CPUs"
>>
>> ?
> 
> Yeah that could be a good idea, I'll try it.

It can come later.

Thanks,

C.