[Qemu-devel] [PATCH v4] target/ppc: Allow PIR read in privileged mode

Greg Kurz posted 1 patch 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/152845121577.13960.4480474870896921301.stgit@bahia.lan
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test s390x passed
target/ppc/translate_init.inc.c |    2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[Qemu-devel] [PATCH v4] target/ppc: Allow PIR read in privileged mode
Posted by Greg Kurz 5 years, 10 months ago
From: luporl <leandro.lupori@gmail.com>

According to PowerISA, the PIR register should be readable in privileged
mode also, not only in hypervisor privileged mode.

PowerISA 3.0 - 4.3.3 Processor Identification Register

"Read access to the PIR is privileged; write access is not provided."

Figure 18 in section 4.4.4 explicitly confirms that mfspr PIR is privileged
and doesn't require hypervisor state.

Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Alexander Graf <agraf@suse.de>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Leandro Lupori <leandro.lupori@gmail.com>
Reviewed-by: Jose Ricardo Ziviani <joserz@linux.ibm.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
Changes in v2:
- added my Signed-off-by, maintainers CC and Jose's Reviewed-by tags

Changes in v3:
- added subsystem name, version tag and summary of changes
- added the section of PowerISA that describes PIR access privileges

Changes in v4 (Greg):
- rebased against ppc-for-3.0 (ie, file is now target/ppc/translate_init.inc.c)
- added some more context from PowerISA
---
 target/ppc/translate_init.inc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index 1a89017ddea8..bb9296f5a3da 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -7819,7 +7819,7 @@ static void gen_spr_book3s_ids(CPUPPCState *env)
     /* Processor identification */
     spr_register_hv(env, SPR_PIR, "PIR",
                  SPR_NOACCESS, SPR_NOACCESS,
-                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, SPR_NOACCESS,
                  &spr_read_generic, NULL,
                  0x00000000);
     spr_register_hv(env, SPR_HID0, "HID0",


Re: [Qemu-devel] [PATCH v4] target/ppc: Allow PIR read in privileged mode
Posted by David Gibson 5 years, 10 months ago
On Fri, Jun 08, 2018 at 11:46:55AM +0200, Greg Kurz wrote:
> From: luporl <leandro.lupori@gmail.com>
> 
> According to PowerISA, the PIR register should be readable in privileged
> mode also, not only in hypervisor privileged mode.
> 
> PowerISA 3.0 - 4.3.3 Processor Identification Register
> 
> "Read access to the PIR is privileged; write access is not provided."
> 
> Figure 18 in section 4.4.4 explicitly confirms that mfspr PIR is privileged
> and doesn't require hypervisor state.

Applied, thanks.

> 
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Leandro Lupori <leandro.lupori@gmail.com>
> Reviewed-by: Jose Ricardo Ziviani <joserz@linux.ibm.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> Changes in v2:
> - added my Signed-off-by, maintainers CC and Jose's Reviewed-by tags
> 
> Changes in v3:
> - added subsystem name, version tag and summary of changes
> - added the section of PowerISA that describes PIR access privileges
> 
> Changes in v4 (Greg):
> - rebased against ppc-for-3.0 (ie, file is now target/ppc/translate_init.inc.c)
> - added some more context from PowerISA
> ---
>  target/ppc/translate_init.inc.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index 1a89017ddea8..bb9296f5a3da 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -7819,7 +7819,7 @@ static void gen_spr_book3s_ids(CPUPPCState *env)
>      /* Processor identification */
>      spr_register_hv(env, SPR_PIR, "PIR",
>                   SPR_NOACCESS, SPR_NOACCESS,
> -                 SPR_NOACCESS, SPR_NOACCESS,
> +                 &spr_read_generic, SPR_NOACCESS,
>                   &spr_read_generic, NULL,
>                   0x00000000);
>      spr_register_hv(env, SPR_HID0, "HID0",
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson