Skip to content

Commit

Permalink
powerpc: Cleanup KVM emulated load/store endian handling
Browse files Browse the repository at this point in the history
Sometimes the KVM code on powerpc needs to emulate load or store
instructions from the guest, which can include both normal and byte
reversed forms.

We currently (AFAICT) handle this correctly, but some variable names are
very misleading.  In particular we use "is_bigendian" in several places to
actually mean "is the IO the same endian as the host", but we now support
little-endian powerpc hosts.  This also ties into the misleadingly named
ld_le*() and st_le*() functions, which in fact always byteswap, even on
an LE host.

This patch cleans this up by renaming to more accurate "host_swabbed", and
uses the generic swab*() functions instead of the powerpc specific and
misleadingly named ld_le*() and st_le*() functions.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
  • Loading branch information
David Gibson authored and Benjamin Herrenschmidt committed Mar 24, 2015
1 parent 7a8bf87 commit d078eed
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 21 deletions.
2 changes: 1 addition & 1 deletion arch/powerpc/include/asm/kvm_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ struct kvm_vcpu_arch {
pgd_t *pgdir;

u8 io_gpr; /* GPR used as IO source/target */
u8 mmio_is_bigendian;
u8 mmio_host_swabbed;
u8 mmio_sign_extend;
u8 osi_needed;
u8 osi_enabled;
Expand Down
38 changes: 18 additions & 20 deletions arch/powerpc/kvm/powerpc.c
Original file line number Diff line number Diff line change
Expand Up @@ -720,18 +720,18 @@ static void kvmppc_complete_mmio_load(struct kvm_vcpu *vcpu,
return;
}

if (vcpu->arch.mmio_is_bigendian) {
if (!vcpu->arch.mmio_host_swabbed) {
switch (run->mmio.len) {
case 8: gpr = *(u64 *)run->mmio.data; break;
case 4: gpr = *(u32 *)run->mmio.data; break;
case 2: gpr = *(u16 *)run->mmio.data; break;
case 1: gpr = *(u8 *)run->mmio.data; break;
}
} else {
/* Convert BE data from userland back to LE. */
switch (run->mmio.len) {
case 4: gpr = ld_le32((u32 *)run->mmio.data); break;
case 2: gpr = ld_le16((u16 *)run->mmio.data); break;
case 8: gpr = swab64(*(u64 *)run->mmio.data); break;
case 4: gpr = swab32(*(u32 *)run->mmio.data); break;
case 2: gpr = swab16(*(u16 *)run->mmio.data); break;
case 1: gpr = *(u8 *)run->mmio.data; break;
}
}
Expand Down Expand Up @@ -780,14 +780,13 @@ int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu,
int is_default_endian)
{
int idx, ret;
int is_bigendian;
bool host_swabbed;

/* Pity C doesn't have a logical XOR operator */
if (kvmppc_need_byteswap(vcpu)) {
/* Default endianness is "little endian". */
is_bigendian = !is_default_endian;
host_swabbed = is_default_endian;
} else {
/* Default endianness is "big endian". */
is_bigendian = is_default_endian;
host_swabbed = !is_default_endian;
}

if (bytes > sizeof(run->mmio.data)) {
Expand All @@ -800,7 +799,7 @@ int kvmppc_handle_load(struct kvm_run *run, struct kvm_vcpu *vcpu,
run->mmio.is_write = 0;

vcpu->arch.io_gpr = rt;
vcpu->arch.mmio_is_bigendian = is_bigendian;
vcpu->arch.mmio_host_swabbed = host_swabbed;
vcpu->mmio_needed = 1;
vcpu->mmio_is_write = 0;
vcpu->arch.mmio_sign_extend = 0;
Expand Down Expand Up @@ -840,14 +839,13 @@ int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu,
{
void *data = run->mmio.data;
int idx, ret;
int is_bigendian;
bool host_swabbed;

/* Pity C doesn't have a logical XOR operator */
if (kvmppc_need_byteswap(vcpu)) {
/* Default endianness is "little endian". */
is_bigendian = !is_default_endian;
host_swabbed = is_default_endian;
} else {
/* Default endianness is "big endian". */
is_bigendian = is_default_endian;
host_swabbed = !is_default_endian;
}

if (bytes > sizeof(run->mmio.data)) {
Expand All @@ -862,19 +860,19 @@ int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu,
vcpu->mmio_is_write = 1;

/* Store the value at the lowest bytes in 'data'. */
if (is_bigendian) {
if (!host_swabbed) {
switch (bytes) {
case 8: *(u64 *)data = val; break;
case 4: *(u32 *)data = val; break;
case 2: *(u16 *)data = val; break;
case 1: *(u8 *)data = val; break;
}
} else {
/* Store LE value into 'data'. */
switch (bytes) {
case 4: st_le32(data, val); break;
case 2: st_le16(data, val); break;
case 1: *(u8 *)data = val; break;
case 8: *(u64 *)data = swab64(val); break;
case 4: *(u32 *)data = swab32(val); break;
case 2: *(u16 *)data = swab16(val); break;
case 1: *(u8 *)data = val; break;
}
}

Expand Down

0 comments on commit d078eed

Please sign in to comment.