Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Implements h_read hcall
On Fri, Feb 15, 2013 at 08:59:35AM -0200, Erlon Cruz wrote:
From: Erlon Cruz erlon.c...@br.flextronics.com
This h_call is useful for DLPAR in future amongst other things. Given an index
it fetches the corresponding PTE stored in the htab.
Signed-off-by: Erlon Cruz erlon.c...@br.flextronics.com
---
hw/spapr_hcall.c | 34 ++
1 file changed, 34 insertions(+)
diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 2889742..1065277 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -323,6 +323,39 @@ static target_ulong h_protect(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
return H_SUCCESS;
}
+static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
+target_ulong opcode, target_ulong *args)
+{
+CPUPPCState *env = cpu-env;
+target_ulong flags = args[0];
+target_ulong pte_index = args[1];
+target_ulong v[4], r[4];
+uint8_t *hpte;
+int i, ridx, n_entries = 1;
+
+if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) {
+return H_PARAMETER;
+}
+
+if (flags H_READ_4) {
+/* Clear the two low order bits */
+pte_index = ~(3ULL);
+n_entries = 4;
+}
+
+hpte = env-external_htab + (pte_index * HASH_PTE_SIZE_64);
+
+for (i = 0, ridx = 0; i n_entries; i++) {
+v[i] = ldq_p(hpte);
+r[i] = ldq_p(hpte + (HASH_PTE_SIZE_64/2));
There's no need for the v and r arrays. You can just need temporaries
for one entry as you store them one-by-one into args.
Otherwise looks good.
--
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
signature.asc
Description: Digital signature
Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Implements h_read hcall
I left them only to make it easy to read and keep the same sintax used
in the other functions.
On Sat, Feb 16, 2013 at 9:46 PM, David Gibson d...@au1.ibm.com wrote:
On Fri, Feb 15, 2013 at 08:59:35AM -0200, Erlon Cruz wrote:
From: Erlon Cruz erlon.c...@br.flextronics.com
This h_call is useful for DLPAR in future amongst other things. Given an
index
it fetches the corresponding PTE stored in the htab.
Signed-off-by: Erlon Cruz erlon.c...@br.flextronics.com
---
hw/spapr_hcall.c | 34 ++
1 file changed, 34 insertions(+)
diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 2889742..1065277 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -323,6 +323,39 @@ static target_ulong h_protect(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
return H_SUCCESS;
}
+static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
+target_ulong opcode, target_ulong *args)
+{
+CPUPPCState *env = cpu-env;
+target_ulong flags = args[0];
+target_ulong pte_index = args[1];
+target_ulong v[4], r[4];
+uint8_t *hpte;
+int i, ridx, n_entries = 1;
+
+if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) {
+return H_PARAMETER;
+}
+
+if (flags H_READ_4) {
+/* Clear the two low order bits */
+pte_index = ~(3ULL);
+n_entries = 4;
+}
+
+hpte = env-external_htab + (pte_index * HASH_PTE_SIZE_64);
+
+for (i = 0, ridx = 0; i n_entries; i++) {
+v[i] = ldq_p(hpte);
+r[i] = ldq_p(hpte + (HASH_PTE_SIZE_64/2));
There's no need for the v and r arrays. You can just need temporaries
for one entry as you store them one-by-one into args.
Otherwise looks good.
--
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
Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Implements h_read hcall
On Sat, Feb 16, 2013 at 11:42:35PM -0300, Erlon Cruz wrote: I left them only to make it easy to read and keep the same sintax used in the other functions. I don't see how having the arrays helps either of those goals. -- 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 signature.asc Description: Digital signature
Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Implements h_read hcall
On Sun, Feb 10, 2013 at 10:10 PM, David Gibson
da...@gibson.dropbear.id.au wrote:
On Thu, Feb 07, 2013 at 09:28:20AM -0200, Erlon Cruz wrote:
From: Erlon Cruz erlon.c...@br.flextronics.com
This h_call is useful for DLPAR in future amongst other things. Given an
index
it fetches the corresponding PTE stored in the htab.
Nice. It would be good to add in this little bit of PAPR compliance.
Couple of small nits in the implementation:
Signed-off-by: Erlon Cruz erlon.c...@br.flextronics.com
---
hw/spapr_hcall.c | 58
++
1 file changed, 58 insertions(+)
diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 2889742..5ba07e5 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -323,6 +323,63 @@ static target_ulong h_protect(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
return H_SUCCESS;
}
+static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
+target_ulong opcode, target_ulong *args)
+{
+CPUPPCState *env = cpu-env;
+target_ulong flags = args[0];
+target_ulong pte_index = args[1];
+uint8_t *hpte;
+
+if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) {
+return H_PARAMETER;
+}
+
+if (!(flags H_READ_4)) {
It would be nice to combine the H_READ_4 and !H_READ_4 paths together,
since except for the masking and stopping sooner the !H_READ_4 path is
just like the H_READ_4 path.
Ok.
+target_ulong v, r;
+target_ulong *pteh = args[0];
+target_ulong *ptel = args[1];
+
+hpte = env-external_htab + (pte_index * HASH_PTE_SIZE_64);
+
+v = ldq_p(hpte);
+r = ldq_p(hpte + (HASH_PTE_SIZE_64/2));
+
+if (flags H_R_XLATE) {
+/* FIXME: include a valid logical page num in the pte */
This comment is misleading. Since you do copy out both words of the
hpte, and qemu stores the external_htab in terms of guest physical (==
logical) addresses, in fact you're *always* supplying a valid logical
page num. So you've already correctly implemented the flag as a
no-op.
I believe that flag is included for the benefit of a true hypervisor,
where the native htab would be stored as true physical addresses, and
it might be expensive for the hypervisor to recompute the logical
address.
Ok, then I think we can just skip this checking.
That said, I actually wrote such helpers about 15 minutes ago as part
of my MMU cleanup series.
Should I wait for the helpers to send It again?
--
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
Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Implements h_read hcall
On Thu, Feb 14, 2013 at 10:44:34AM -0200, Erlon Cruz wrote:
On Sun, Feb 10, 2013 at 10:10 PM, David Gibson
da...@gibson.dropbear.id.au wrote:
On Thu, Feb 07, 2013 at 09:28:20AM -0200, Erlon Cruz wrote:
From: Erlon Cruz erlon.c...@br.flextronics.com
This h_call is useful for DLPAR in future amongst other things. Given an
index
it fetches the corresponding PTE stored in the htab.
Nice. It would be good to add in this little bit of PAPR compliance.
Couple of small nits in the implementation:
Signed-off-by: Erlon Cruz erlon.c...@br.flextronics.com
---
hw/spapr_hcall.c | 58
++
1 file changed, 58 insertions(+)
diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 2889742..5ba07e5 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -323,6 +323,63 @@ static target_ulong h_protect(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
return H_SUCCESS;
}
+static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
+target_ulong opcode, target_ulong *args)
+{
+CPUPPCState *env = cpu-env;
+target_ulong flags = args[0];
+target_ulong pte_index = args[1];
+uint8_t *hpte;
+
+if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) {
+return H_PARAMETER;
+}
+
+if (!(flags H_READ_4)) {
It would be nice to combine the H_READ_4 and !H_READ_4 paths together,
since except for the masking and stopping sooner the !H_READ_4 path is
just like the H_READ_4 path.
Ok.
+target_ulong v, r;
+target_ulong *pteh = args[0];
+target_ulong *ptel = args[1];
+
+hpte = env-external_htab + (pte_index * HASH_PTE_SIZE_64);
+
+v = ldq_p(hpte);
+r = ldq_p(hpte + (HASH_PTE_SIZE_64/2));
+
+if (flags H_R_XLATE) {
+/* FIXME: include a valid logical page num in the pte */
This comment is misleading. Since you do copy out both words of the
hpte, and qemu stores the external_htab in terms of guest physical (==
logical) addresses, in fact you're *always* supplying a valid logical
page num. So you've already correctly implemented the flag as a
no-op.
I believe that flag is included for the benefit of a true hypervisor,
where the native htab would be stored as true physical addresses, and
it might be expensive for the hypervisor to recompute the logical
address.
Ok, then I think we can just skip this checking.
Yes.
That said, I actually wrote such helpers about 15 minutes ago as part
of my MMU cleanup series.
Should I wait for the helpers to send It again?
Probably not, my series will be a little while because it will
probably need to wait for the big cpu qomification series.
--
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
signature.asc
Description: Digital signature
Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Implements h_read hcall
On 13.02.2013, at 06:21, David Gibson wrote:
On Tue, Feb 12, 2013 at 11:07:10PM +0100, Alexander Graf wrote:
On 07.02.2013, at 12:28, Erlon Cruz wrote:
From: Erlon Cruz erlon.c...@br.flextronics.com
This h_call is useful for DLPAR in future amongst other things. Given an
index
it fetches the corresponding PTE stored in the htab.
Signed-off-by: Erlon Cruz erlon.c...@br.flextronics.com
---
hw/spapr_hcall.c | 58
++
1 file changed, 58 insertions(+)
diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 2889742..5ba07e5 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -323,6 +323,63 @@ static target_ulong h_protect(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
return H_SUCCESS;
}
+static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
+target_ulong opcode, target_ulong *args)
+{
+CPUPPCState *env = cpu-env;
+target_ulong flags = args[0];
+target_ulong pte_index = args[1];
+uint8_t *hpte;
+
+if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) {
+return H_PARAMETER;
+}
+
+if (!(flags H_READ_4)) {
+target_ulong v, r;
+target_ulong *pteh = args[0];
+target_ulong *ptel = args[1];
+
+hpte = env-external_htab + (pte_index * HASH_PTE_SIZE_64);
You are not guaranteed that there is an external htab.
Actually in the case of spapr, you are - the existing hash table
management calls all assume the existence of an external htab.
Ok, just leave the code using external_htab and we'll make it use the helpers
once they're there.
Alex
In fact, looking at the external_htab users, we should probably
introduce a few helper read functions for the htab that abstract the
glorious external_htab/htab_base details away from you.
That said, I actually wrote such helpers about 15 minutes ago as part
of my MMU cleanup series.
--
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
Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Implements h_read hcall
On Tue, Feb 12, 2013 at 11:07:10PM +0100, Alexander Graf wrote:
On 07.02.2013, at 12:28, Erlon Cruz wrote:
From: Erlon Cruz erlon.c...@br.flextronics.com
This h_call is useful for DLPAR in future amongst other things. Given an
index
it fetches the corresponding PTE stored in the htab.
Signed-off-by: Erlon Cruz erlon.c...@br.flextronics.com
---
hw/spapr_hcall.c | 58
++
1 file changed, 58 insertions(+)
diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 2889742..5ba07e5 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -323,6 +323,63 @@ static target_ulong h_protect(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
return H_SUCCESS;
}
+static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
+target_ulong opcode, target_ulong *args)
+{
+CPUPPCState *env = cpu-env;
+target_ulong flags = args[0];
+target_ulong pte_index = args[1];
+uint8_t *hpte;
+
+if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) {
+return H_PARAMETER;
+}
+
+if (!(flags H_READ_4)) {
+target_ulong v, r;
+target_ulong *pteh = args[0];
+target_ulong *ptel = args[1];
+
+hpte = env-external_htab + (pte_index * HASH_PTE_SIZE_64);
You are not guaranteed that there is an external htab.
Actually in the case of spapr, you are - the existing hash table
management calls all assume the existence of an external htab.
In fact, looking at the external_htab users, we should probably
introduce a few helper read functions for the htab that abstract the
glorious external_htab/htab_base details away from you.
That said, I actually wrote such helpers about 15 minutes ago as part
of my MMU cleanup series.
--
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
signature.asc
Description: Digital signature
Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Implements h_read hcall
On Thu, Feb 07, 2013 at 09:28:20AM -0200, Erlon Cruz wrote:
From: Erlon Cruz erlon.c...@br.flextronics.com
This h_call is useful for DLPAR in future amongst other things. Given an index
it fetches the corresponding PTE stored in the htab.
Nice. It would be good to add in this little bit of PAPR compliance.
Couple of small nits in the implementation:
Signed-off-by: Erlon Cruz erlon.c...@br.flextronics.com
---
hw/spapr_hcall.c | 58
++
1 file changed, 58 insertions(+)
diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 2889742..5ba07e5 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -323,6 +323,63 @@ static target_ulong h_protect(PowerPCCPU *cpu,
sPAPREnvironment *spapr,
return H_SUCCESS;
}
+static target_ulong h_read(PowerPCCPU *cpu, sPAPREnvironment *spapr,
+target_ulong opcode, target_ulong *args)
+{
+CPUPPCState *env = cpu-env;
+target_ulong flags = args[0];
+target_ulong pte_index = args[1];
+uint8_t *hpte;
+
+if ((pte_index * HASH_PTE_SIZE_64) ~env-htab_mask) {
+return H_PARAMETER;
+}
+
+if (!(flags H_READ_4)) {
It would be nice to combine the H_READ_4 and !H_READ_4 paths together,
since except for the masking and stopping sooner the !H_READ_4 path is
just like the H_READ_4 path.
+target_ulong v, r;
+target_ulong *pteh = args[0];
+target_ulong *ptel = args[1];
+
+hpte = env-external_htab + (pte_index * HASH_PTE_SIZE_64);
+
+v = ldq_p(hpte);
+r = ldq_p(hpte + (HASH_PTE_SIZE_64/2));
+
+if (flags H_R_XLATE) {
+/* FIXME: include a valid logical page num in the pte */
This comment is misleading. Since you do copy out both words of the
hpte, and qemu stores the external_htab in terms of guest physical (==
logical) addresses, in fact you're *always* supplying a valid logical
page num. So you've already correctly implemented the flag as a
no-op.
I believe that flag is included for the benefit of a true hypervisor,
where the native htab would be stored as true physical addresses, and
it might be expensive for the hypervisor to recompute the logical
address.
--
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
signature.asc
Description: Digital signature
