Re: [Qemu-devel] [PATCH 08/11] target/arm: Add guarded_pages cpu property for user-only

2019-01-28 Thread Richard Henderson
On 1/22/19 8:57 AM, Peter Maydell wrote:
>>> This is OK code-wise but I'm a bit wary of committing it
>>> because then we're stuck with the property forever even
>>> if it turns out to be irrelevant to whatever the userland
>>> ABI eventually is.
>>
>> That's surely simply a matter of documentation.
>> "Don't rely on this property: it will go away."
> 
> Convention is that those should be prefixed with "x-".

Ok, done.


r~



Re: [Qemu-devel] [PATCH 08/11] target/arm: Add guarded_pages cpu property for user-only

2019-01-22 Thread Richard Henderson
On 1/22/19 7:29 AM, Peter Maydell wrote:
> On Thu, 10 Jan 2019 at 12:18, Richard Henderson
>  wrote:
>>
>> While waiting for a proper userland ABI, allow static test
>> cases to be written assuming that GP is set for all pages.
>>
>> Signed-off-by: Richard Henderson 
>> ---
>>  target/arm/cpu.h   |  4 
>>  target/arm/cpu64.c | 18 ++
>>  target/arm/translate-a64.c |  8 +++-
>>  3 files changed, 29 insertions(+), 1 deletion(-)
> 
> This is OK code-wise but I'm a bit wary of committing it
> because then we're stuck with the property forever even
> if it turns out to be irrelevant to whatever the userland
> ABI eventually is.

That's surely simply a matter of documentation.
"Don't rely on this property: it will go away."


r~



Re: [Qemu-devel] [PATCH 08/11] target/arm: Add guarded_pages cpu property for user-only

2019-01-22 Thread Peter Maydell
On Thu, 10 Jan 2019 at 12:18, Richard Henderson
 wrote:
>
> While waiting for a proper userland ABI, allow static test
> cases to be written assuming that GP is set for all pages.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/cpu.h   |  4 
>  target/arm/cpu64.c | 18 ++
>  target/arm/translate-a64.c |  8 +++-
>  3 files changed, 29 insertions(+), 1 deletion(-)

This is OK code-wise but I'm a bit wary of committing it
because then we're stuck with the property forever even
if it turns out to be irrelevant to whatever the userland
ABI eventually is.

thanks
-- PMM



Re: [Qemu-devel] [PATCH 08/11] target/arm: Add guarded_pages cpu property for user-only

2019-01-22 Thread Peter Maydell
On Tue, 22 Jan 2019 at 15:42, Richard Henderson
 wrote:
>
> On 1/22/19 7:29 AM, Peter Maydell wrote:
> > On Thu, 10 Jan 2019 at 12:18, Richard Henderson
> >  wrote:
> >>
> >> While waiting for a proper userland ABI, allow static test
> >> cases to be written assuming that GP is set for all pages.
> >>
> >> Signed-off-by: Richard Henderson 
> >> ---
> >>  target/arm/cpu.h   |  4 
> >>  target/arm/cpu64.c | 18 ++
> >>  target/arm/translate-a64.c |  8 +++-
> >>  3 files changed, 29 insertions(+), 1 deletion(-)
> >
> > This is OK code-wise but I'm a bit wary of committing it
> > because then we're stuck with the property forever even
> > if it turns out to be irrelevant to whatever the userland
> > ABI eventually is.
>
> That's surely simply a matter of documentation.
> "Don't rely on this property: it will go away."

Convention is that those should be prefixed with "x-".

thanks
-- PMM



[Qemu-devel] [PATCH 08/11] target/arm: Add guarded_pages cpu property for user-only

2019-01-10 Thread Richard Henderson
While waiting for a proper userland ABI, allow static test
cases to be written assuming that GP is set for all pages.

Signed-off-by: Richard Henderson 
---
 target/arm/cpu.h   |  4 
 target/arm/cpu64.c | 18 ++
 target/arm/translate-a64.c |  8 +++-
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 506c490a16..929f16dd6b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -882,6 +882,10 @@ struct ARMCPU {
  */
 bool cfgend;
 
+#ifdef CONFIG_USER_ONLY
+bool guarded_pages;
+#endif
+
 QLIST_HEAD(, ARMELChangeHook) pre_el_change_hooks;
 QLIST_HEAD(, ARMELChangeHook) el_change_hooks;
 
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index d0de0d5dcf..713d2d5579 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -315,6 +315,18 @@ static void cpu_max_set_packey(Object *obj, Visitor *v, 
const char *name,
 }
 error_propagate(errp, err);
 }
+
+static bool aarch64_cpu_get_guarded_pages(Object *obj, Error **errp)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+return cpu->guarded_pages;
+}
+
+static void aarch64_cpu_set_guarded_pages(Object *obj, bool val, Error **errp)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+cpu->guarded_pages = val;
+}
 #endif
 
 /* -cpu max: if KVM is enabled, like -cpu host (best possible with this host);
@@ -420,6 +432,12 @@ static void aarch64_max_initfn(Object *obj)
 cpu->env.cp15.sctlr_el[1] |= SCTLR_EnIA | SCTLR_EnIB;
 cpu->env.cp15.sctlr_el[1] |= SCTLR_EnDA | SCTLR_EnDB;
 }
+
+object_property_add_bool(obj, "guarded_pages",
+ aarch64_cpu_get_guarded_pages,
+ aarch64_cpu_set_guarded_pages, NULL);
+object_property_set_description(obj, "guarded_pages",
+"Set on/off GuardPage bit for all pages", NULL);
 #endif
 
 cpu->sve_max_vq = ARM_MAX_VQ;
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 5f0ecb297f..f225517077 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -13780,7 +13780,13 @@ static void disas_data_proc_simd_fp(DisasContext *s, 
uint32_t insn)
 static bool is_guarded_page(CPUARMState *env, DisasContext *s)
 {
 #ifdef CONFIG_USER_ONLY
-return false;  /* FIXME */
+/*
+ * FIXME: What is the userland ABI for this?
+ * For the moment this is controlled by an attribute:
+ *   -cpu max,guarded_pages=on.
+ */
+ARMCPU *cpu = arm_env_get_cpu(env);
+return cpu->guarded_pages;
 #else
 uint64_t addr = s->base.pc_first;
 int mmu_idx = arm_to_core_mmu_idx(s->mmu_idx);
-- 
2.17.2