[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] target/ppc: fix Hash64 MMU update of PTE bit R

From: Cédric Le Goater
Subject: Re: [PATCH v3] target/ppc: fix Hash64 MMU update of PTE bit R
Date: Mon, 29 Nov 2021 13:00:09 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.0

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 163c90388a..8ebf85bad8 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1414,7 +1414,7 @@ void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
           kvmppc_write_hpte(ptex, pte0, pte1);
       } else {
           if (pte0 & HPTE64_V_VALID) {
-            stq_p(spapr->htab + offset + HASH_PTE_SIZE_64 / 2, pte1);
+            stq_p(spapr->htab + offset + HPTE64_R_BYTE_OFFSET, pte1);

Urgh.. so, initially I thought this was wrong because I was confusing
HPTE64_R_BYTE_OFFSET with HPTE64_R_R_BYTE_OFFSET.  I doubt I'd be the
only one.

Calling something a BYTE_OFFSET then doing an stq to it is pretty
misleading I think.  WORD1_OFFSET or R_WORD_OFFSET might be better?

How about (inspired from XIVE) :

  #define HPTE64_W1    (HASH_PTE_SIZE_64 / 2)
  #define HPTE64_W1_R  14 // or HPTE64_W1 + 6
  #define HPTE64_W1_C  15 // or HPTE64_W1 + 7

Good enough.

But a word is 32bits in Power. Let's use DW1 to be consistent with the
architecture. I think this is less confusing than using W2.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]