On 3/31/21 5:06 AM, David Gibson wrote:

On Tue, Mar 30, 2021 at 06:48:38PM +0200, Greg Kurz wrote:

quoted

On Tue, 30 Mar 2021 15:23:50 +0530
Ravi Bangoria [off-list ref] wrote:

quoted

As per the PAPR, bit 0 of byte 64 in pa-features property indicates
availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to
find whether kvm supports 2nd DAWR or not. If it's supported, allow
user to set the pa-feature bit in guest DT using cap-dawr1 machine
capability. Though, watchpoint on powerpc TCG guest is not supported
and thus 2nd DAWR is not enabled for TCG mode.

Signed-off-by: Ravi Bangoria <redacted>
---

LGTM. A couple of remarks, see below.

quoted

  hw/ppc/spapr.c                  | 11 ++++++++++-
  hw/ppc/spapr_caps.c             | 32 ++++++++++++++++++++++++++++++++
  include/hw/ppc/spapr.h          |  6 +++++-
  target/ppc/cpu.h                |  2 ++
  target/ppc/kvm.c                | 12 ++++++++++++
  target/ppc/kvm_ppc.h            |  7 +++++++
  target/ppc/translate_init.c.inc | 15 +++++++++++++++
  7 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d56418ca29..4660ff9e6b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c

@@ -238,7 +238,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
          0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
          /* 54: DecFP, 56: DecI, 58: SHA */
          0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
-        /* 60: NM atomic, 62: RNG */
+        /* 60: NM atomic, 62: RNG, 64: DAWR1 (ISA 3.1) */
          0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
      };
      uint8_t *pa_features = NULL;

@@ -256,6 +256,10 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
          pa_features = pa_features_300;
          pa_size = sizeof(pa_features_300);
      }
+    if (ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_10, 0, cpu->compat_pvr)) {
+        pa_features = pa_features_300;
+        pa_size = sizeof(pa_features_300);
+    }

This isn't strictly needed right now because a POWER10 processor has
PCR_COMPAT_3_00, so the previous ppc_check_compat() block sets
pa_features to pa_features300 already. I guess this will make sense
when/if POWER10 has its own pa_features_310 one day.

This should be removed for now.  We're definitely too late for
qemu-6.0 at this point, so might as well polish this.

The rest of Greg's comments look like they're good, too.

Sure. Will respin with these changes.

Thanks for the review,
Ravi