[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [RFC PATCH 25/49] ARM: new VGIC: Add GICv2 world switch backend
Hi Andre,
On 02/26/2018 03:13 PM, Andre Przywara wrote:
Hi,
On 13/02/18 14:31, Julien Grall wrote:
Hi,
On 09/02/18 14:39, Andre Przywara wrote:
Processing maintenance interrupts and accessing the list registers
are dependent on the host's GIC version.
Introduce vgic-v2.c to contain GICv2 specific functions.
Implement the GICv2 specific code for syncing the emulation state
into the VGIC registers.
This also adds the hook to let Xen setup the host GIC addresses.
This is based on Linux commit 140b086dd197, written by Marc Zyngier.
Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
---
xen/arch/arm/vgic/vgic-v2.c | 261
++++++++++++++++++++++++++++++++++++++++++++
xen/arch/arm/vgic/vgic.c | 20 ++++
xen/arch/arm/vgic/vgic.h | 8 ++
3 files changed, 289 insertions(+)
create mode 100644 xen/arch/arm/vgic/vgic-v2.c
diff --git a/xen/arch/arm/vgic/vgic-v2.c b/xen/arch/arm/vgic/vgic-v2.c
new file mode 100644
index 0000000000..10fc467ffa
--- /dev/null
+++ b/xen/arch/arm/vgic/vgic-v2.c
@@ -0,0 +1,261 @@
+/*
+ * Copyright (C) 2015, 2016 ARM Ltd.
+ * Imported from Linux ("new" KVM VGIC) and heavily adapted to Xen.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <asm/arm_vgic.h>
+#include <asm/bug.h>
+#include <asm/io.h>
+#include <xen/sched.h>
+#include <xen/sizes.h>
+
+#include "vgic.h"
+
+#define GICH_ELRSR0 0x30
+#define GICH_ELRSR1 0x34
+#define GICH_LR0 0x100
+
+#define GICH_LR_VIRTUALID (0x3ff << 0)
+#define GICH_LR_PHYSID_CPUID_SHIFT (10)
+#define GICH_LR_PHYSID_CPUID (0x3ff <<
GICH_LR_PHYSID_CPUID_SHIFT)
+#define GICH_LR_PRIORITY_SHIFT 23
+#define GICH_LR_STATE (3 << 28)
+#define GICH_LR_PENDING_BIT (1 << 28)
+#define GICH_LR_ACTIVE_BIT (1 << 29)
+#define GICH_LR_EOI (1 << 19)
+#define GICH_LR_HW (1 << 31)
Can we define them in either in gic.h or a new header gic-v2.h?
Yes, but they clash with some ill-named GICv3 LR bits. So expect another
patch which renames GICH_LR_STATE_SHIFT to ICH_LR_STATE_SHIFT. Which is
the actual spec name for that system register in GICv3, there is no
GICH_LR_ with the GICv3 bit positions.
While this would be a nice clean-up. Wouldn't create a new gic-v2.h
sufficient?
+
+static struct {
+ bool enabled;
+ paddr_t dbase; /* Distributor interface address */
+ paddr_t cbase; /* CPU interface address & size */
+ paddr_t csize;
+ paddr_t vbase; /* Virtual CPU interface address */
+ void __iomem *hbase; /* Hypervisor control interface */
+
+ /* Offset to add to get an 8kB contiguous region if GIC is
aliased */
+ uint32_t aliased_offset;
+} gic_v2_hw_data;
+
+void vgic_v2_setup_hw(paddr_t dbase, paddr_t cbase, paddr_t csize,
+ paddr_t vbase, void __iomem *hbase,
+ uint32_t aliased_offset)
+{
+ gic_v2_hw_data.enabled = true;
+ gic_v2_hw_data.dbase = dbase;
+ gic_v2_hw_data.cbase = cbase;
+ gic_v2_hw_data.csize = csize;
+ gic_v2_hw_data.vbase = vbase;
+ gic_v2_hw_data.hbase = hbase;
+ gic_v2_hw_data.aliased_offset = aliased_offset;
+}
+
+void vgic_v2_set_underflow(struct vcpu *vcpu)
+{
+ gic_hw_ops->update_hcr_status(GICH_HCR_UIE, 1);
+}
+
+/*
+ * transfer the content of the LRs back into the corresponding ap_list:
+ * - active bit is transferred as is
+ * - pending bit is
+ * - transferred as is in case of edge sensitive IRQs
+ * - set to the line-level (resample time) for level sensitive IRQs
+ */
+void vgic_v2_fold_lr_state(struct vcpu *vcpu)
I am wondering how much we could share this code with
vgic_v3_fold_lr_state.
I think we discussed this and dismissed the idea:
- The actual LR encoding is much different between GICv3 and GICv2, up
to the point where we have some fields in one which are not in the
other. That really clutters the code.
- Originally this function was much shorter and didn't have that many
special cases. So the code duplication was really minimal.
I see your point, but don't really want to go there now for two reasons:
- It is probably nasty to implement, since we always have to check which
GIC we are running on when masking the LR value.
- It would deviate further from the KVM implementation, in a core
function. For any bugs introduced we are on our own here.
I will try to bring this up with the KVM people, to see whether it's
worth to revisit this decision. There is indeed quite some code
duplication these days.
But this may come as an optimization later.
Fine with me. It was mostly to avoid having to review twice the same
hairy code.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
