[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v2 02/17] arm64: vgic-v3: Add ICV_BPR1_EL1 handler
On 03/27/2018 05:08 PM, Marc Zyngier wrote:
On 27/03/18 12:27, Manish Jaggi wrote:
On 03/27/2018 04:55 PM, Marc Zyngier wrote:
On 27/03/18 12:15, Manish Jaggi wrote:
On 03/27/2018 04:41 PM, Marc Zyngier wrote:
On 27/03/18 12:07, Manish Jaggi wrote:
On 03/27/2018 04:35 PM, Marc Zyngier wrote:
On 27/03/18 11:56, Manish Jaggi wrote:
On 03/27/2018 04:15 PM, Marc Zyngier wrote:
On 27/03/18 11:35, Manish Jaggi wrote:
On 03/27/2018 04:00 PM, Marc Zyngier wrote:
On 27/03/18 10:07, Manish Jaggi wrote:
This patch is ported to xen from linux commit
d70c7b31a60f2458f35c226131f2a01a7a98b6cf
KVM: arm64: vgic-v3: Add ICV_BPR1_EL1 handler
Add a handler for reading/writing the guest's view of the ICC_BPR1_EL1
register, which is located in the ICH_VMCR_EL2.BPR1 field.
Signed-off-by: Manish Jaggi <manish.jaggi@xxxxxxxxxx>
---
xen/arch/arm/arm64/vgic-v3-sr.c | 70
+++++++++++++++++++++++++++++++++++++
xen/include/asm-arm/arm64/sysregs.h | 1 +
xen/include/asm-arm/gic_v3_defs.h | 6 ++++
3 files changed, 77 insertions(+)
diff --git a/xen/arch/arm/arm64/vgic-v3-sr.c b/xen/arch/arm/arm64/vgic-v3-sr.c
index 39ab1ed6ca..ed4254acf9 100644
--- a/xen/arch/arm/arm64/vgic-v3-sr.c
+++ b/xen/arch/arm/arm64/vgic-v3-sr.c
@@ -18,10 +18,76 @@
*/
#include <asm/current.h>
+#include <asm/gic_v3_defs.h>
#include <asm/regs.h>
#include <asm/system.h>
#include <asm/traps.h>
+#define vtr_to_nr_pre_bits(v) ((((uint32_t)(v) >> 26) & 7) + 1)
+
+static int vgic_v3_bpr_min(void)
+{
+ /* See Pseudocode for VPriorityGroup */
+ return 8 - vtr_to_nr_pre_bits(READ_SYSREG32(ICH_VTR_EL2));
+}
+
+static unsigned int vgic_v3_get_bpr0(uint32_t vmcr)
+{
+ return (vmcr & ICH_VMCR_BPR0_MASK) >> ICH_VMCR_BPR0_SHIFT;
+}
+
+static unsigned int vgic_v3_get_bpr1(uint32_t vmcr)
+{
+ unsigned int bpr;
+
+ if ( vmcr & ICH_VMCR_CBPR_MASK )
+ {
+ bpr = vgic_v3_get_bpr0(vmcr);
+ if ( bpr < 7 )
+ bpr++;
+ }
+ else
+ bpr = (vmcr & ICH_VMCR_BPR1_MASK) >> ICH_VMCR_BPR1_SHIFT;
+
+ return bpr;
+}
+
+static void vgic_v3_read_bpr1(struct cpu_user_regs *regs, int regidx)
+{
+ uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+
+ set_user_reg(regs, regidx, vgic_v3_get_bpr1(vmcr));
+}
+
+static void vgic_v3_write_bpr1(struct cpu_user_regs *regs, int regidx)
+{
+ register_t val = get_user_reg(regs, regidx);
+ uint8_t bpr_min = vgic_v3_bpr_min();
+ uint32_t vmcr = READ_SYSREG32(ICH_VMCR_EL2);
+
+ if ( vmcr & ICH_VMCR_CBPR_MASK )
+ return;
+
+ /* Enforce BPR limiting */
+ if ( val < bpr_min )
+ val = bpr_min;
+
+ val <<= ICH_VMCR_BPR1_SHIFT;
+ val &= ICH_VMCR_BPR1_MASK;
+ vmcr &= ~ICH_VMCR_BPR1_MASK;
+ vmcr |= val;
+
+ WRITE_SYSREG32(vmcr, ICH_VMCR_EL2);
+}
+
+static void vreg_emulate_bpr1(struct cpu_user_regs *regs, const union hsr hsr)
+{
+ if ( hsr.sysreg.read )
+ vgic_v3_read_bpr1(regs, hsr.sysreg.reg);
+ else
+ vgic_v3_write_bpr1(regs, hsr.sysreg.reg);
+}
+
/*
* returns true if the register is emulated.
*/
@@ -40,6 +106,10 @@ bool vgic_v3_handle_cpuif_access(struct cpu_user_regs *regs)
switch ( hsr.bits & HSR_SYSREG_REGS_MASK )
{
+ case HSR_SYSREG_ICC_BPR1_EL1:
+ vreg_emulate_bpr1(regs, hsr);
+ break;
What is the rational for indirecting through a function and moving the
reading of VMCR to the leaf functions? I appreciate that this doesn't
change much, but since this is a port of existing code, it will make
more complex the port of potential fixes.
I used xen template of handling sysreg traps
If you see the file xen/arch/arm/arm64/sysreg.c
a handle_XXX function is used throughout...
Sure, but this is not sysreg.c. This is a separate file for a reason
(i.e. it is imported code). Anyway, that's for the Xen maintainers to
decide.
More importantly, my other question still stand: most trap functions do
require VMCR as an input. Why moving it to the leaf functions?
Same reason, I was keeping the interface same of all handle_XXX functions
handle_XXX(regs, hsr, ...)
Do you want me to change both to match with your patch or it is ok?
My preference would be to keep the code as initially written, as you're
pointlessly changing the flow. Again, that's for the maintainers to
comment, but you should at the very least indicate that change in the
commit log.
I did in the cover letter
which, crucially, doesn't end-up in the commit. Oh well.
I am working on to address the two points, will send v3 later today.
- will remove emulate functions
- and pass vmcr as a param.
Will it be possible to review the remaining part of the code, so that I
can address
other comments in v3 as well.
I suggest you wait until some other folks have a chance to properly
review the series. You only posted the stuff this morning, give them a
chance. A week between two versions is probably the right timing.
Xen 4.11 window closes this week, so I have only few days.
I am hoping to get your ack before that.
My Ack is not the maintainers' (which you'd need anyway), and your
series is not the only one I need to review. I don't plan on having
another look at it this week anyway.
As for the Xen deadline, I'm sure there will be another one.
Hi Julien,
Is it prudent to send v3 with the two changes of marc suggested in this
mail ?
Since this is a errata workaround, and hopefully it should make it to 4.11.
I have tried to address the v1 comments in v2, could you please have a
look at it.
Thanks
manish
M.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
