Re: [Xen-devel] [PATCH v2 03/25] arm/altp2m: Add struct vttbr.
On 08/06/2016 03:20 PM, Julien Grall wrote:
>
>
> On 06/08/2016 09:54, Sergej Proskurin wrote:
>> Hi Julien,
>
> Hello Sergej,
>
>> On 08/04/2016 06:15 PM, Julien Grall wrote:
>>>
>>>
>>> On 04/08/16 17:11, Sergej Proskurin wrote:
>>> diff --git a/xen/include/asm-arm/processor.h
>>> b/xen/include/asm-arm/processor.h
>>> index 15bf890..f8ca18c 100644
>>> --- a/xen/include/asm-arm/processor.h
>>> +++ b/xen/include/asm-arm/processor.h
>>> @@ -529,6 +529,22 @@ union hsr {
>>>
>>>
>>> };
>>> +
>>> +/* VTTBR: Virtualization Translation Table Base Register */
>>> +struct vttbr {
>>> +union {
>>> +struct {
>>> +u64 baddr :40, /* variable res0: from 0-(x-1) bit */
>>
>> As mentioned on the previous series, this field is 48 bits for ARMv8
>> (see ARM D7.2.102 in DDI 0487A.j).
>>
I must have missed it during refactoring. At this point, I will
distinguish between __arm__ and __aarch64__, thank you.
>>>
>>> After reading this series I see no point having this union. So I would
>>> much prefer to see this patch dropped.
>>>
>>
>> I can do that. However, I do not understand why we would prefer using
>> error prone bit operations for VTTBR initialization instead of having a
>> unified and simple way of initializing and using the VTTBR including the
>> VMID and the root table address.
>
> The VTTBR only needs to be initialized in one place and we don't care
> accessing the fields. So I don't see the benefit to introduce a
> structure for that.
>
Ok. I will drop this patch.
Best regards,
~Sergej
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 03/25] arm/altp2m: Add struct vttbr.
On 06/08/2016 09:54, Sergej Proskurin wrote:
Hi Julien,
Hello Sergej,
On 08/04/2016 06:15 PM, Julien Grall wrote:
On 04/08/16 17:11, Sergej Proskurin wrote:
diff --git a/xen/include/asm-arm/processor.h
b/xen/include/asm-arm/processor.h
index 15bf890..f8ca18c 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -529,6 +529,22 @@ union hsr {
};
+
+/* VTTBR: Virtualization Translation Table Base Register */
+struct vttbr {
+union {
+struct {
+u64 baddr :40, /* variable res0: from 0-(x-1) bit */
As mentioned on the previous series, this field is 48 bits for ARMv8
(see ARM D7.2.102 in DDI 0487A.j).
I must have missed it during refactoring. At this point, I will
distinguish between __arm__ and __aarch64__, thank you.
After reading this series I see no point having this union. So I would
much prefer to see this patch dropped.
I can do that. However, I do not understand why we would prefer using
error prone bit operations for VTTBR initialization instead of having a
unified and simple way of initializing and using the VTTBR including the
VMID and the root table address.
The VTTBR only needs to be initialized in one place and we don't care
accessing the fields. So I don't see the benefit to introduce a
structure for that.
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 03/25] arm/altp2m: Add struct vttbr.
Hi Julien,
On 08/04/2016 06:15 PM, Julien Grall wrote:
>
>
> On 04/08/16 17:11, Sergej Proskurin wrote:
> diff --git a/xen/include/asm-arm/processor.h
> b/xen/include/asm-arm/processor.h
> index 15bf890..f8ca18c 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -529,6 +529,22 @@ union hsr {
>
>
> };
> +
> +/* VTTBR: Virtualization Translation Table Base Register */
> +struct vttbr {
> +union {
> +struct {
> +u64 baddr :40, /* variable res0: from 0-(x-1) bit */
As mentioned on the previous series, this field is 48 bits for ARMv8
(see ARM D7.2.102 in DDI 0487A.j).
>>
>> I must have missed it during refactoring. At this point, I will
>> distinguish between __arm__ and __aarch64__, thank you.
>
> After reading this series I see no point having this union. So I would
> much prefer to see this patch dropped.
>
I can do that. However, I do not understand why we would prefer using
error prone bit operations for VTTBR initialization instead of having a
unified and simple way of initializing and using the VTTBR including the
VMID and the root table address.
Best regards,
~Sergej
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 03/25] arm/altp2m: Add struct vttbr.
On 04/08/16 17:11, Sergej Proskurin wrote:
diff --git a/xen/include/asm-arm/processor.h
b/xen/include/asm-arm/processor.h
index 15bf890..f8ca18c 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -529,6 +529,22 @@ union hsr {
};
+
+/* VTTBR: Virtualization Translation Table Base Register */
+struct vttbr {
+union {
+struct {
+u64 baddr :40, /* variable res0: from 0-(x-1) bit */
As mentioned on the previous series, this field is 48 bits for ARMv8
(see ARM D7.2.102 in DDI 0487A.j).
I must have missed it during refactoring. At this point, I will
distinguish between __arm__ and __aarch64__, thank you.
After reading this series I see no point having this union. So I would
much prefer to see this patch dropped.
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 03/25] arm/altp2m: Add struct vttbr.
Hi Julien,
>> Hello Sergej,
>>
>> Title: s/altp2m/p2m/
I will adapt the titles of all patches, thank you.
>>
>> On 01/08/16 18:10, Sergej Proskurin wrote:
>>> The struct vttbr introduces a simple way to precisely access the
>>> individual fields of the vttbr.
>>
>> I am not sure whether this is really helpful. You don't seem to take
>> often advantage of those fields and the actual accesses don't seem
>> necessary (I will comment on the usage).
>>
>>> ---
>>> xen/arch/arm/p2m.c | 8
>>> xen/arch/arm/traps.c| 2 +-
>>> xen/include/asm-arm/p2m.h | 2 +-
>>> xen/include/asm-arm/processor.h | 16
>>> 4 files changed, 22 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>>> index 40a0b80..cbc64a1 100644
>>> --- a/xen/arch/arm/p2m.c
>>> +++ b/xen/arch/arm/p2m.c
>>> @@ -122,7 +122,7 @@ void p2m_restore_state(struct vcpu *n)
>>> WRITE_SYSREG(hcr & ~HCR_VM, HCR_EL2);
>>> isb();
>>>
>>> -WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
>>> +WRITE_SYSREG64(p2m->vttbr.vttbr, VTTBR_EL2);
>>> isb();
>>>
>>> if ( is_32bit_domain(n->domain) )
>>> @@ -147,10 +147,10 @@ static void p2m_flush_tlb(struct p2m_domain *p2m)
>>> * VMID. So switch to the VTTBR of a given P2M if different.
>>> */
>>> ovttbr = READ_SYSREG64(VTTBR_EL2);
>>> -if ( ovttbr != p2m->vttbr )
>>> +if ( ovttbr != p2m->vttbr.vttbr )
>>> {
>>> local_irq_save(flags);
>>> -WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
>>> +WRITE_SYSREG64(p2m->vttbr.vttbr, VTTBR_EL2);
>>> isb();
>>> }
>>>
>>> @@ -1293,7 +1293,7 @@ static int p2m_alloc_table(struct domain *d)
>>>
>>> p2m->root = page;
>>>
>>> -p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid &
>>> 0xff) << 48;
>>> +p2m->vttbr.vttbr = page_to_maddr(p2m->root) |
>>> ((uint64_t)p2m->vmid & 0xff) << 48;
>>>
>>> /*
>>> * Make sure that all TLBs corresponding to the new VMID are
>>> flushed
>>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>>> index 06f06e3..12be7c9 100644
>>> --- a/xen/arch/arm/traps.c
>>> +++ b/xen/arch/arm/traps.c
>>> @@ -881,7 +881,7 @@ void vcpu_show_registers(const struct vcpu *v)
>>> ctxt.ifsr32_el2 = v->arch.ifsr;
>>> #endif
>>>
>>> -ctxt.vttbr_el2 = v->domain->arch.p2m.vttbr;
>>> +ctxt.vttbr_el2 = v->domain->arch.p2m.vttbr.vttbr;
>>>
>>> _show_registers(>arch.cpu_info->guest_cpu_user_regs, , 1,
>>> v);
>>> }
>>> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
>>> index 53c4d78..5c7cd1a 100644
>>> --- a/xen/include/asm-arm/p2m.h
>>> +++ b/xen/include/asm-arm/p2m.h
>>> @@ -33,7 +33,7 @@ struct p2m_domain {
>>> uint8_t vmid;
>>>
>>> /* Current Translation Table Base Register for the p2m */
>>> -uint64_t vttbr;
>>> +struct vttbr vttbr;
>>>
>>> /*
>>> * Highest guest frame that's ever been mapped in the p2m
>>> diff --git a/xen/include/asm-arm/processor.h
>>> b/xen/include/asm-arm/processor.h
>>> index 15bf890..f8ca18c 100644
>>> --- a/xen/include/asm-arm/processor.h
>>> +++ b/xen/include/asm-arm/processor.h
>>> @@ -529,6 +529,22 @@ union hsr {
>>>
>>>
>>> };
>>> +
>>> +/* VTTBR: Virtualization Translation Table Base Register */
>>> +struct vttbr {
>>> +union {
>>> +struct {
>>> +u64 baddr :40, /* variable res0: from 0-(x-1) bit */
>>
>> As mentioned on the previous series, this field is 48 bits for ARMv8
>> (see ARM D7.2.102 in DDI 0487A.j).
>>
I must have missed it during refactoring. At this point, I will
distinguish between __arm__ and __aarch64__, thank you.
>>> +res1 :8,
>>> +vmid :8,
>>> +res2 :8;
>>> +};
>>> +u64 vttbr;
>>> +};
>>> +};
>>> +
>>> +#define INVALID_VTTBR (0UL)
>>> +
>>> #endif
>>>
>>> /* HSR.EC == HSR_CP{15,14,10}_32 */
>>>
>>
>> Regards,
>>
>
Best regards,
~Sergej
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 03/25] arm/altp2m: Add struct vttbr.
(CC Stefano)
On 03/08/16 18:04, Julien Grall wrote:
Hello Sergej,
Title: s/altp2m/p2m/
On 01/08/16 18:10, Sergej Proskurin wrote:
The struct vttbr introduces a simple way to precisely access the
individual fields of the vttbr.
I am not sure whether this is really helpful. You don't seem to take
often advantage of those fields and the actual accesses don't seem
necessary (I will comment on the usage).
---
xen/arch/arm/p2m.c | 8
xen/arch/arm/traps.c| 2 +-
xen/include/asm-arm/p2m.h | 2 +-
xen/include/asm-arm/processor.h | 16
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 40a0b80..cbc64a1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -122,7 +122,7 @@ void p2m_restore_state(struct vcpu *n)
WRITE_SYSREG(hcr & ~HCR_VM, HCR_EL2);
isb();
-WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
+WRITE_SYSREG64(p2m->vttbr.vttbr, VTTBR_EL2);
isb();
if ( is_32bit_domain(n->domain) )
@@ -147,10 +147,10 @@ static void p2m_flush_tlb(struct p2m_domain *p2m)
* VMID. So switch to the VTTBR of a given P2M if different.
*/
ovttbr = READ_SYSREG64(VTTBR_EL2);
-if ( ovttbr != p2m->vttbr )
+if ( ovttbr != p2m->vttbr.vttbr )
{
local_irq_save(flags);
-WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
+WRITE_SYSREG64(p2m->vttbr.vttbr, VTTBR_EL2);
isb();
}
@@ -1293,7 +1293,7 @@ static int p2m_alloc_table(struct domain *d)
p2m->root = page;
-p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid &
0xff) << 48;
+p2m->vttbr.vttbr = page_to_maddr(p2m->root) |
((uint64_t)p2m->vmid & 0xff) << 48;
/*
* Make sure that all TLBs corresponding to the new VMID are flushed
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 06f06e3..12be7c9 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -881,7 +881,7 @@ void vcpu_show_registers(const struct vcpu *v)
ctxt.ifsr32_el2 = v->arch.ifsr;
#endif
-ctxt.vttbr_el2 = v->domain->arch.p2m.vttbr;
+ctxt.vttbr_el2 = v->domain->arch.p2m.vttbr.vttbr;
_show_registers(>arch.cpu_info->guest_cpu_user_regs, , 1,
v);
}
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 53c4d78..5c7cd1a 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -33,7 +33,7 @@ struct p2m_domain {
uint8_t vmid;
/* Current Translation Table Base Register for the p2m */
-uint64_t vttbr;
+struct vttbr vttbr;
/*
* Highest guest frame that's ever been mapped in the p2m
diff --git a/xen/include/asm-arm/processor.h
b/xen/include/asm-arm/processor.h
index 15bf890..f8ca18c 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -529,6 +529,22 @@ union hsr {
};
+
+/* VTTBR: Virtualization Translation Table Base Register */
+struct vttbr {
+union {
+struct {
+u64 baddr :40, /* variable res0: from 0-(x-1) bit */
As mentioned on the previous series, this field is 48 bits for ARMv8
(see ARM D7.2.102 in DDI 0487A.j).
+res1 :8,
+vmid :8,
+res2 :8;
+};
+u64 vttbr;
+};
+};
+
+#define INVALID_VTTBR (0UL)
+
#endif
/* HSR.EC == HSR_CP{15,14,10}_32 */
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 03/25] arm/altp2m: Add struct vttbr.
Hello Sergej,
Title: s/altp2m/p2m/
On 01/08/16 18:10, Sergej Proskurin wrote:
The struct vttbr introduces a simple way to precisely access the
individual fields of the vttbr.
I am not sure whether this is really helpful. You don't seem to take
often advantage of those fields and the actual accesses don't seem
necessary (I will comment on the usage).
---
xen/arch/arm/p2m.c | 8
xen/arch/arm/traps.c| 2 +-
xen/include/asm-arm/p2m.h | 2 +-
xen/include/asm-arm/processor.h | 16
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 40a0b80..cbc64a1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -122,7 +122,7 @@ void p2m_restore_state(struct vcpu *n)
WRITE_SYSREG(hcr & ~HCR_VM, HCR_EL2);
isb();
-WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
+WRITE_SYSREG64(p2m->vttbr.vttbr, VTTBR_EL2);
isb();
if ( is_32bit_domain(n->domain) )
@@ -147,10 +147,10 @@ static void p2m_flush_tlb(struct p2m_domain *p2m)
* VMID. So switch to the VTTBR of a given P2M if different.
*/
ovttbr = READ_SYSREG64(VTTBR_EL2);
-if ( ovttbr != p2m->vttbr )
+if ( ovttbr != p2m->vttbr.vttbr )
{
local_irq_save(flags);
-WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
+WRITE_SYSREG64(p2m->vttbr.vttbr, VTTBR_EL2);
isb();
}
@@ -1293,7 +1293,7 @@ static int p2m_alloc_table(struct domain *d)
p2m->root = page;
-p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) << 48;
+p2m->vttbr.vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff)
<< 48;
/*
* Make sure that all TLBs corresponding to the new VMID are flushed
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 06f06e3..12be7c9 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -881,7 +881,7 @@ void vcpu_show_registers(const struct vcpu *v)
ctxt.ifsr32_el2 = v->arch.ifsr;
#endif
-ctxt.vttbr_el2 = v->domain->arch.p2m.vttbr;
+ctxt.vttbr_el2 = v->domain->arch.p2m.vttbr.vttbr;
_show_registers(>arch.cpu_info->guest_cpu_user_regs, , 1, v);
}
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 53c4d78..5c7cd1a 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -33,7 +33,7 @@ struct p2m_domain {
uint8_t vmid;
/* Current Translation Table Base Register for the p2m */
-uint64_t vttbr;
+struct vttbr vttbr;
/*
* Highest guest frame that's ever been mapped in the p2m
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 15bf890..f8ca18c 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -529,6 +529,22 @@ union hsr {
};
+
+/* VTTBR: Virtualization Translation Table Base Register */
+struct vttbr {
+union {
+struct {
+u64 baddr :40, /* variable res0: from 0-(x-1) bit */
As mentioned on the previous series, this field is 48 bits for ARMv8
(see ARM D7.2.102 in DDI 0487A.j).
+res1 :8,
+vmid :8,
+res2 :8;
+};
+u64 vttbr;
+};
+};
+
+#define INVALID_VTTBR (0UL)
+
#endif
/* HSR.EC == HSR_CP{15,14,10}_32 */
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 03/25] arm/altp2m: Add struct vttbr.
The struct vttbr introduces a simple way to precisely access the
individual fields of the vttbr.
---
xen/arch/arm/p2m.c | 8
xen/arch/arm/traps.c| 2 +-
xen/include/asm-arm/p2m.h | 2 +-
xen/include/asm-arm/processor.h | 16
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 40a0b80..cbc64a1 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -122,7 +122,7 @@ void p2m_restore_state(struct vcpu *n)
WRITE_SYSREG(hcr & ~HCR_VM, HCR_EL2);
isb();
-WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
+WRITE_SYSREG64(p2m->vttbr.vttbr, VTTBR_EL2);
isb();
if ( is_32bit_domain(n->domain) )
@@ -147,10 +147,10 @@ static void p2m_flush_tlb(struct p2m_domain *p2m)
* VMID. So switch to the VTTBR of a given P2M if different.
*/
ovttbr = READ_SYSREG64(VTTBR_EL2);
-if ( ovttbr != p2m->vttbr )
+if ( ovttbr != p2m->vttbr.vttbr )
{
local_irq_save(flags);
-WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
+WRITE_SYSREG64(p2m->vttbr.vttbr, VTTBR_EL2);
isb();
}
@@ -1293,7 +1293,7 @@ static int p2m_alloc_table(struct domain *d)
p2m->root = page;
-p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff) << 48;
+p2m->vttbr.vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid & 0xff)
<< 48;
/*
* Make sure that all TLBs corresponding to the new VMID are flushed
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 06f06e3..12be7c9 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -881,7 +881,7 @@ void vcpu_show_registers(const struct vcpu *v)
ctxt.ifsr32_el2 = v->arch.ifsr;
#endif
-ctxt.vttbr_el2 = v->domain->arch.p2m.vttbr;
+ctxt.vttbr_el2 = v->domain->arch.p2m.vttbr.vttbr;
_show_registers(>arch.cpu_info->guest_cpu_user_regs, , 1, v);
}
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 53c4d78..5c7cd1a 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -33,7 +33,7 @@ struct p2m_domain {
uint8_t vmid;
/* Current Translation Table Base Register for the p2m */
-uint64_t vttbr;
+struct vttbr vttbr;
/*
* Highest guest frame that's ever been mapped in the p2m
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 15bf890..f8ca18c 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -529,6 +529,22 @@ union hsr {
};
+
+/* VTTBR: Virtualization Translation Table Base Register */
+struct vttbr {
+union {
+struct {
+u64 baddr :40, /* variable res0: from 0-(x-1) bit */
+res1 :8,
+vmid :8,
+res2 :8;
+};
+u64 vttbr;
+};
+};
+
+#define INVALID_VTTBR (0UL)
+
#endif
/* HSR.EC == HSR_CP{15,14,10}_32 */
--
2.9.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
