Re: [Qemu-devel] [PATCH] Enable support for PPC64 on qemu target v2

[Laurent Vivier]

Le 27 févr. 09 à 00:39, Alexander Graf a écrit :

> This patch enables OpenBIOS to initialize on PPC64, enabling support  
> for -cpu 970fx.
> It gets up to the boot prompt and works rather good so far, though I  
> haven't been able to run a kernel yet.
>
> For more recent PowerPC CPUs the PTE layout has changed, so we need  
> to take that into account and create PTEs according to the new  
> layout and at the new physical positions.
>
> v2 takes Laurent's comments into account

Thank you

> Signed-off-by: Alexander Graf <address@hidden>
...
> Index: arch/ppc/qemu/mmutypes.h
> ===================================================================
> --- arch/ppc/qemu/mmutypes.h    (revision 459)
> +++ arch/ppc/qemu/mmutypes.h    (working copy)
> @@ -38,7 +38,28 @@
>         unsigned long pp:2;     /* Page protection */
>  } mPTE_t;
>
> +typedef struct mPTE_64 {
> +       uint32_t avpn_low;      /* Abbreviated Virtual Page Number (unused) */
> +       uint32_t avpn:25;       /* Abbreviated Virtual Page Number */
> +       uint32_t sw:4;          /* Software Use */
> +       uint32_t  :1;           /* Reserved */
> +       uint32_t h:1;           /* Hash algorithm indicator */
> +       uint32_t v:1;           /* Entry is valid */
>
> +       uint32_t rpn_low;       /* Real (physical) page number (unused) */
> +       uint32_t rpn:20;        /* Real (physical) page number */
> +       uint32_t    :2;         /* Reserved */
> +       uint32_t ac:1;          /* Address Compare*/
> +       uint32_t r:1;           /* Referenced */
> +       uint32_t c:1;           /* Changed */
> +       uint32_t w:1;           /* Write-thru cache mode */
> +       uint32_t i:1;           /* Cache inhibited */
> +       uint32_t m:1;           /* Memory coherence */
> +       uint32_t g:1;           /* Guarded */
> +       uint32_t n:1;           /* No-Execute */
> +       uint32_t pp:2;          /* Page protection */
> +} mPTE_64_t;
> +

To answer to the comment of your previous patch:

Porting openBIOS to ppc64 seems useless as ppc32 can run on ppc64 (Do  
you want to run a database in openBIOS ? ;-) ). If I remember  
correctly there were some mails about this between Aurélien and  
BlueSwirl on openBIOS mailing list.

But I agree that "unsigned long"is not  a good choice in this case.

And a personal comment you must ignore: I don't like bitfield.

But if you use mask/shift and defines, you can use same values for 32  
and 64bit PTE (it's not true for all fields):

#define PTE_PAGE_PROTECTION 0x0003
#define PTE_NOEXEC  0x0004
...
#define PTE_CHANGED   0x0100
...
#define PTE_RPN 0xfffffffffffff000ULL

But as it is done now is not an issue for me, it's just, from a  
personal and cosmetic point of view, ugly...

I'll try to test and commit your patch tomorrow (well, tomorrow is  
today, now...)

Regards,
Laurent