[Qemu-devel] [PATCH RFC] target-ppc: Correctly handle translation address when bus unit ID = 0x07F

[Qemu-devel] [PATCH RFC] target-ppc: Correctly handle translation address when bus unit ID = 0x07F

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

In that case, we want to access memory space instead of I/O controller
interface address space.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

Simplify by avoiding reindentation of existing code.

Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 Hello Alex,
 
 This patch fixes a hang when booting 40p, please review.
 
 The only vaguely related section in Power ISA 2.06B I could find was the
 Programming Note on p. 764 (5.7.1).
 6xx_pem.pdf identifies the masked SR bits as Bus unit ID, says nothing
 about the 0x07f value though - might that be machine-specific?
 
 Andreas
 
 target-ppc/helper.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index cf2a368..cdf8d15 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -949,8 +949,18 @@ static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
             ret = -3;
         }
     } else {
+        target_ulong sr;
         LOG_MMU("direct store...\n");
         /* Direct-store segment : absolutely *BUGGY* for now */
+
+        sr = env->sr[eaddr >> 28];
+        if ((sr & 0x1FF00000) >> 20 == 0x07f) {
+            /* Memory forced */
+            ctx->raddr = ((sr & 0xF) << 28) | (eaddr & 0x0FFFFFFF);
+            ctx->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+            return 0;
+        }
+
         switch (type) {
         case ACCESS_INT:
             /* Integer load/store : only access allowed */
-- 
1.7.5.3

Re: [Qemu-devel] [PATCH RFC] target-ppc: Correctly handle translation address when bus unit ID = 0x07F

[Alexander Graf]

On 13.06.2011, at 12:13, Andreas Färber wrote:

> From: Hervé Poussineau <hpoussin@reactos.org>
> 
> In that case, we want to access memory space instead of I/O controller
> interface address space.
> 
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> 
> Simplify by avoiding reindentation of existing code.
> 
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
> Hello Alex,
> 
> This patch fixes a hang when booting 40p, please review.
> 
> The only vaguely related section in Power ISA 2.06B I could find was the
> Programming Note on p. 764 (5.7.1).

The 601 is not 2.06 compliant, so you need to dig up earlier manuals. Google revealed this for the 601: 

  http://www.freescale.com/files/32bit/doc/user_guide/MPC601UM.pdf

> 6xx_pem.pdf identifies the masked SR bits as Bus unit ID, says nothing
> about the 0x07f value though - might that be machine-specific?
> 
> Andreas
> 
> target-ppc/helper.c |   10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
> index cf2a368..cdf8d15 100644
> --- a/target-ppc/helper.c
> +++ b/target-ppc/helper.c
> @@ -949,8 +949,18 @@ static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
>             ret = -3;
>         }
>     } else {
> +        target_ulong sr;
>         LOG_MMU("direct store...\n");

This is direct store, so you're in T=1

>         /* Direct-store segment : absolutely *BUGGY* for now */
> +
> +        sr = env->sr[eaddr >> 28];
> +        if ((sr & 0x1FF00000) >> 20 == 0x07f) {

This is the BUID, yes.
According to page 70 in the manual I mentioned above, the following passage applies:


Memory-forced I/O controller interface (BUID = x'07F')—Memory-forced I/O controller interface operations access memory space. They do not use the extensions to the memory protocol described for I/O controller interface accesses, and they bypass the page- and block-translation and protection mechanisms. The physical address is found by concatenating bits 28–31 of the respective segment register with bits 4–31 of the effective address. This address is marked as noncacheable, write- through, and global.
Because memory-forced I/O controller interface accesses address memory space, they are subject to the same coherency control as other memory reference operations. More generally, accesses to memory-forced I/O controller interface segments are considered to be cache-inhibited, write-through and memory-coherent operations with respect to the 601 cache and bus interface.


Since we don't implement any cache ourselves and MMIO is simply handled immediately, I don't think there's anything special that needs to be done, except for mapping it as EA=RA.

> +            /* Memory forced */
> +            ctx->raddr = ((sr & 0xF) << 28) | (eaddr & 0x0FFFFFFF);

This is exactly the same as ctx->raddr = eaddr, no?

So yes, in general the patch looks fine. Please add some comments to document what's going on though. I don't think everyone who reads the code wants to dig the out from the manuals :).



Alex

[Qemu-devel] [PATCH v2] target-ppc: Handle memory-forced I/O controller access

[Andreas Färber]

From: Hervé Poussineau <hpoussin@reactos.org>

On at least the PowerPC 601, a direct-store (T=1) with bus unit ID 0x07F
is special-cased as memory-forced I/O controller access. It is supposed
to be checked immediately if T=1, bypassing all protection mechanisms
and acting cache-inhibited and global.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

Simplified by avoiding reindentation. Added explanatory comments.

Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 target-ppc/helper.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index cf2a368..2944b06 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -949,8 +949,24 @@ static inline int get_segment(CPUState *env, mmu_ctx_t *ctx,
             ret = -3;
         }
     } else {
+        target_ulong sr;
         LOG_MMU("direct store...\n");
         /* Direct-store segment : absolutely *BUGGY* for now */
+
+        /* Direct-store implies a 32-bit MMU.
+         * Check the Segment Register's bus unit ID (BUID).
+         */
+        sr = env->sr[eaddr >> 28];
+        if ((sr & 0x1FF00000) >> 20 == 0x07f) {
+            /* Memory-forced I/O controller interface access */
+            /* If T=1 and BUID=x'07F', the 601 performs a memory access
+             * to SR[28-31] LA[4-31], bypassing all protection mechanisms.
+             */
+            ctx->raddr = ((sr & 0xF) << 28) | (eaddr & 0x0FFFFFFF);
+            ctx->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+            return 0;
+        }
+
         switch (type) {
         case ACCESS_INT:
             /* Integer load/store : only access allowed */
-- 
1.7.5.3

Re: [Qemu-devel] [PATCH v2] target-ppc: Handle memory-forced I/O controller access

[Alexander Graf]

On 14.06.2011, at 23:27, Andreas Färber wrote:

> From: Hervé Poussineau <hpoussin@reactos.org>
> 
> On at least the PowerPC 601, a direct-store (T=1) with bus unit ID 0x07F
> is special-cased as memory-forced I/O controller access. It is supposed
> to be checked immediately if T=1, bypassing all protection mechanisms
> and acting cache-inhibited and global.

Thanks, applied to ppc-next.


Alex

Re: [Qemu-devel] [PATCH RFC] target-ppc: Correctly handle translation address when bus unit ID = 0x07F

[Andreas Färber]

Am 13.06.2011 um 15:31 schrieb Alexander Graf:

> On 13.06.2011, at 12:13, Andreas Färber wrote:
>
>> +            /* Memory forced */
>> +            ctx->raddr = ((sr & 0xF) << 28) | (eaddr & 0x0FFFFFFF);
>
> This is exactly the same as ctx->raddr = eaddr, no?

No, not that I see. The manual is explicit about this calculation:

"the processor [...] generates a memory access
with the physical address specified by the lowest-order four bits in  
the segment register
(SR[28–31]) concatenated with LA4–LA31." (6-63 / 6.10.4)

That matches figure 6-26 on the following page, and I've added a  
similar comment in v2.

I see no compelling reason why the lower nibble of the SR should  
always match the SR#, the upper nibble of eaddr.

Andreas

Re: [Qemu-devel] [PATCH RFC] target-ppc: Correctly handle translation address when bus unit ID = 0x07F

[Alexander Graf]

On 14.06.2011, at 23:49, Andreas Färber wrote:

> Am 13.06.2011 um 15:31 schrieb Alexander Graf:
> 
>> On 13.06.2011, at 12:13, Andreas Färber wrote:
>> 
>>> +            /* Memory forced */
>>> +            ctx->raddr = ((sr & 0xF) << 28) | (eaddr & 0x0FFFFFFF);
>> 
>> This is exactly the same as ctx->raddr = eaddr, no?
> 
> No, not that I see. The manual is explicit about this calculation:
> 
> "the processor [...] generates a memory access
> with the physical address specified by the lowest-order four bits in the segment register
> (SR[28–31]) concatenated with LA4–LA31." (6-63 / 6.10.4)
> 
> That matches figure 6-26 on the following page, and I've added a similar comment in v2.
> 
> I see no compelling reason why the lower nibble of the SR should always match the SR#, the upper nibble of eaddr.

Ah, sr is not the segment register number, but the content of the segement register. Then it's ok :).


Alex