From: christoffer.dall@linaro.org (Christoffer Dall)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 04/19] arm/arm64: KVM: wrap 64 bit MMIO accesses with two 32 bit ones
Date: Tue, 4 Nov 2014 14:24:23 +0100 [thread overview]
Message-ID: <20141104132423.GB2537@cbox> (raw)
In-Reply-To: <5458C408.5070102@arm.com>
On Tue, Nov 04, 2014 at 12:18:16PM +0000, Andre Przywara wrote:
> Hi Christoffer,
>
> On 03/11/14 13:25, Christoffer Dall wrote:
> > On Fri, Oct 31, 2014 at 05:26:39PM +0000, Andre Przywara wrote:
> >> Some GICv3 registers can and will be accessed as 64 bit registers.
> >> Currently the register handling code can only deal with 32 bit
> >> accesses, so we do two consecutive calls to cover this.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >> ---
> >> virt/kvm/arm/vgic.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
> >> 1 file changed, 45 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> >> index 704be48..0cbdde9 100644
> >> --- a/virt/kvm/arm/vgic.c
> >> +++ b/virt/kvm/arm/vgic.c
> >> @@ -1033,6 +1033,48 @@ static bool vgic_validate_access(const struct vgic_dist *dist,
> >> }
> >>
> >> /*
> >> + * Call the respective handler function for the given range.
> >> + * We split up any 64 bit accesses into two consecutive 32 bit
> >> + * handler calls and merge the result afterwards.
> >> + */
> >> +static bool call_range_handler(struct kvm_vcpu *vcpu,
> >> + struct kvm_exit_mmio *mmio,
> >> + unsigned long offset,
> >> + const struct mmio_range *range)
> >> +{
> >> + u32 *data32 = (void *)mmio->data;
> >> + struct kvm_exit_mmio mmio32;
> >> + bool ret;
> >> +
> >> + if (likely(mmio->len <= 4))
> >> + return range->handle_mmio(vcpu, mmio, offset);
> >> +
> >> + /*
> >> + * Any access bigger than 4 bytes (that we currently handle in KVM)
> >> + * is actually 8 bytes long, caused by a 64-bit access
> >> + */
> >> +
> >> + mmio32.len = 4;
> >> + mmio32.is_write = mmio->is_write;
> >> +
> >> + mmio32.phys_addr = mmio->phys_addr + 4;
> >> + if (mmio->is_write)
> >> + *(u32 *)mmio32.data = data32[1];
> >> + ret = range->handle_mmio(vcpu, &mmio32, offset + 4);
> >> + if (!mmio->is_write)
> >> + data32[1] = *(u32 *)mmio32.data;
> >> +
> >> + mmio32.phys_addr = mmio->phys_addr;
> >> + if (mmio->is_write)
> >> + *(u32 *)mmio32.data = data32[0];
> >> + ret |= range->handle_mmio(vcpu, &mmio32, offset);
> >> + if (!mmio->is_write)
> >> + data32[0] = *(u32 *)mmio32.data;
> >> +
> >> + return ret;
> >> +}
> >
> > Please think about the endianness issues here.
>
> I didn't only think about it, I traced the code and tested it:
> So it works like written above (I actually had a hickup in my kvmtool
> setup that denied booting the bigendian initrds, so I thought that BE
> was broken).
>
> So the GIC is always LE, that's why we swap the bytes to LE in any
> 32-bit register in mmio_data_{write,read}, which gets called for each
> vGIC register access via the vgic_reg_access() function.
>
> So the memory order that the actual register handler functions
> implicitly expect is always LE, regardless of the guest or host
> endianness. vgic_reg_access() makes this transparent for the host code.
>
> Now if we eventually assemble the 64-bit value from the two 32-bit
> values, we also have to always do this in LE fashion. Hence the
> hardcoded LE assignment here. Eventually this LE value will be copied
> into the guest, which will access it through readq, which uses
> le64_to_cpu() to convert it to the CPU native value.
>
> So the branch as posted (or present in the repo) works fine (boot-tested
> only so far) with all 8 combinations of (host endianness, guest
> endianness, guest v2/v3 GIC).
>
> I will add a comment to the function explaining this.
>
Yes, you're right. Thanks for the explanation. I think the key to
understanding that this works is the fact that mmio_data is always
written in LE in memory.
I was thrown off by the conversion you were making to a u32*, which you
don't really use, except as index mamipulation and to copy the data, but
that's fine.
Thanks for explaining this.
-Christoffer
next prev parent reply other threads:[~2014-11-04 13:24 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-31 17:26 [PATCH v3 00/19] KVM GICv3 emulation Andre Przywara
2014-10-31 17:26 ` [PATCH v3 01/19] arm/arm64: KVM: rework MPIDR assignment and add accessors Andre Przywara
2014-11-03 13:13 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 02/19] arm/arm64: KVM: pass down user space provided GIC type into vGIC code Andre Przywara
2014-11-03 13:14 ` Christoffer Dall
2014-11-03 13:25 ` Andre Przywara
2014-11-03 16:51 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 03/19] arm/arm64: KVM: refactor vgic_handle_mmio() function Andre Przywara
2014-11-03 13:23 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 04/19] arm/arm64: KVM: wrap 64 bit MMIO accesses with two 32 bit ones Andre Przywara
2014-11-03 13:25 ` Christoffer Dall
2014-11-04 12:18 ` Andre Przywara
2014-11-04 13:24 ` Christoffer Dall [this message]
2014-10-31 17:26 ` [PATCH v3 05/19] arm/arm64: KVM: introduce per-VM ops Andre Przywara
2014-11-03 13:59 ` Christoffer Dall
2014-11-04 15:58 ` Andre Przywara
2014-11-04 19:03 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 06/19] arm/arm64: KVM: move [sg]et_lr into " Andre Przywara
2014-11-03 14:15 ` Christoffer Dall
2014-11-04 16:30 ` Andre Przywara
2014-11-04 19:12 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 07/19] arm/arm64: KVM: move kvm_register_device_ops() into vGIC probing Andre Przywara
2014-11-03 20:05 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 08/19] arm/arm64: KVM: dont rely on a valid GICH base address Andre Przywara
2014-11-03 20:05 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 09/19] arm/arm64: KVM: make the maximum number of vCPUs a per-VM value Andre Przywara
2014-11-03 20:06 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 10/19] arm/arm64: KVM: make the value of ICC_SRE_EL1 a per-VM variable Andre Przywara
2014-11-03 20:04 ` Christoffer Dall
2014-11-03 20:17 ` Marc Zyngier
2014-11-07 19:18 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 11/19] arm/arm64: KVM: refactor MMIO accessors Andre Przywara
2014-11-04 11:55 ` Christoffer Dall
2014-11-04 12:25 ` Andre Przywara
2014-10-31 17:26 ` [PATCH v3 12/19] arm/arm64: KVM: refactor/wrap vgic_set/get_attr() Andre Przywara
2014-11-04 19:30 ` Christoffer Dall
2014-11-05 10:27 ` Andre Przywara
2014-11-05 10:37 ` Andre Przywara
2014-11-05 12:57 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 13/19] arm/arm64: KVM: add vgic.h header file Andre Przywara
2014-11-04 19:30 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 14/19] arm/arm64: KVM: split GICv2 specific emulation code from vgic.c Andre Przywara
2014-11-04 19:30 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 15/19] arm/arm64: KVM: add opaque private pointer to MMIO accessors Andre Przywara
2014-11-04 15:44 ` Christoffer Dall
2014-11-04 17:24 ` Andre Przywara
2014-11-04 18:05 ` Marc Zyngier
2014-11-04 19:18 ` Christoffer Dall
2014-11-04 20:17 ` Marc Zyngier
2014-11-05 9:49 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 16/19] arm/arm64: KVM: add virtual GICv3 distributor emulation Andre Przywara
2014-11-07 14:30 ` Christoffer Dall
2014-11-10 17:30 ` [PATCH v3 16/19] arm/arm64: KVM: add virtual GICv3 distributor emulation / PART 1 Andre Przywara
2014-11-11 13:48 ` Christoffer Dall
2014-11-12 12:39 ` [PATCH v3 16/19] arm/arm64: KVM: add virtual GICv3 distributor emulation / PART 2 Andre Przywara
2014-11-12 19:51 ` Christoffer Dall
2014-11-13 11:18 ` Christoffer Dall
2014-11-13 11:45 ` Marc Zyngier
2014-11-13 12:01 ` Andre Przywara
2014-10-31 17:26 ` [PATCH v3 17/19] arm64: KVM: add SGI system register trapping Andre Przywara
2014-11-07 15:07 ` Christoffer Dall
2014-11-10 11:31 ` Andre Przywara
2014-11-10 12:45 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 18/19] arm/arm64: KVM: enable kernel side of GICv3 emulation Andre Przywara
2014-11-07 16:07 ` Christoffer Dall
2014-11-10 12:19 ` Andre Przywara
2014-11-10 13:24 ` Christoffer Dall
2014-10-31 17:26 ` [PATCH v3 19/19] arm/arm64: KVM: allow userland to request a virtual GICv3 Andre Przywara
2014-11-07 16:15 ` Christoffer Dall
2014-11-10 12:26 ` Andre Przywara
2014-11-10 13:25 ` Christoffer Dall
2014-11-03 12:59 ` [PATCH v3 00/19] KVM GICv3 emulation Christoffer Dall
2014-11-06 10:57 ` Christoffer Dall
2014-11-06 11:21 ` Christoffer Dall
2014-11-06 15:13 ` Andre Przywara
2014-11-06 18:09 ` Christoffer Dall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141104132423.GB2537@cbox \
--to=christoffer.dall@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).