linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
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

  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).