On Wed, Oct 12, 2016 at 06:13:49PM -0500, Michael Roth wrote: > PAPR guests advertise their capabilities to the platform by passing > an ibm,architecture-vec structure via an > ibm,client-architecture-support hcall as described by LoPAPR v11, > B.6.2.3. during early boot. > > Using this information, the platform enables the capabilities it > supports, then encodes a subset of those enabled capabilities (the > 5th option vector of the ibm,architecture-vec structure passed to > ibm,client-architecture-support) into the guest device tree via > "/chosen/ibm,architecture-vec-5". > > The logical format of these these option vectors is a bit-vector, > where individual bits are addressed/documented based on the byte-wise > offset from the beginning of the bit-vector, followed by the bit-wise > index starting from the byte-wise offset. Thus the bits of each of > these bytes are stored in reverse order. Additionally, the first > byte of each option vector is encodes the length of the option vector, > so byte offsets begin at 1, and bit offset at 0.
Heh.. pity qemu doesn't use the ccan bitmap module (http://ccodearchive.net/info/bitmap.html). By design it always stores the bitmaps in IBM bit number ordering, because that's most obvious to a human reading a memory dump (for the purpose of bit vectors - in most situations the IBM numbering is dumb). > This is not very intuitive for the purposes of mapping these bits to > a particular documented capability, so this patch introduces a set > of abstractions that encapsulate the work of parsing/encoding these > options vectors and testing for individual capabilities. > > Cc: Bharata B Rao <bhar...@linux.vnet.ibm.com> > Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com> A handful of small nits. > --- > hw/ppc/Makefile.objs | 2 +- > hw/ppc/spapr_ovec.c | 244 > ++++++++++++++++++++++++++++++++++++++++++++ > include/hw/ppc/spapr_ovec.h | 62 +++++++++++ > 3 files changed, 307 insertions(+), 1 deletion(-) > create mode 100644 hw/ppc/spapr_ovec.c > create mode 100644 include/hw/ppc/spapr_ovec.h > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > index 99a0d4e..2e0b0c9 100644 > --- a/hw/ppc/Makefile.objs > +++ b/hw/ppc/Makefile.objs > @@ -4,7 +4,7 @@ obj-y += ppc.o ppc_booke.o fdt.o > obj-$(CONFIG_PSERIES) += spapr.o spapr_vio.o spapr_events.o > obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o > obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o spapr_rng.o > -obj-$(CONFIG_PSERIES) += spapr_cpu_core.o > +obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o > ifeq ($(CONFIG_PCI)$(CONFIG_PSERIES)$(CONFIG_LINUX), yyy) > obj-y += spapr_pci_vfio.o > endif > diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c > new file mode 100644 > index 0000000..ddc19f5 > --- /dev/null > +++ b/hw/ppc/spapr_ovec.c > @@ -0,0 +1,244 @@ > +/* > + * QEMU SPAPR Architecture Option Vector Helper Functions > + * > + * Copyright IBM Corp. 2016 > + * > + * Authors: > + * Bharata B Rao <bhar...@linux.vnet.ibm.com> > + * Michael Roth <mdr...@linux.vnet.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > + > +#include "qemu/osdep.h" > +#include "hw/ppc/spapr_ovec.h" > +#include "qemu/bitmap.h" > +#include "exec/address-spaces.h" > +#include "qemu/error-report.h" > +#include <libfdt.h> > + > +/* #define DEBUG_SPAPR_OVEC */ > + > +#ifdef DEBUG_SPAPR_OVEC > +#define DPRINTFN(fmt, ...) \ > + do { fprintf(stderr, fmt "\n", ## __VA_ARGS__); } while (0) > +#else > +#define DPRINTFN(fmt, ...) \ > + do { } while (0) > +#endif > + > +#define OV_MAXBYTES 256 /* not including length byte */ > +#define OV_MAXBITS (OV_MAXBYTES * BITS_PER_BYTE) > + > +/* we *could* work with bitmaps directly, but handling the bitmap privately > + * allows us to more safely make assumptions about the bitmap size and > + * simplify the calling code somewhat > + */ > +struct sPAPROptionVector { > + unsigned long *bitmap; > +}; > + > +static sPAPROptionVector *spapr_ovec_from_bitmap(unsigned long *bitmap) > +{ > + sPAPROptionVector *ov; > + > + g_assert(bitmap); > + > + ov = g_new0(sPAPROptionVector, 1); > + ov->bitmap = bitmap; > + > + return ov; > +} > + > +sPAPROptionVector *spapr_ovec_new(void) > +{ > + return spapr_ovec_from_bitmap(bitmap_new(OV_MAXBITS)); > +} > + > +sPAPROptionVector *spapr_ovec_clone(sPAPROptionVector *ov_orig) > +{ > + sPAPROptionVector *ov; > + > + g_assert(ov_orig); > + > + ov = spapr_ovec_new(); > + bitmap_copy(ov->bitmap, ov_orig->bitmap, OV_MAXBITS); > + > + return ov; > +} > + > +void spapr_ovec_intersect(sPAPROptionVector *ov, > + sPAPROptionVector *ov1, > + sPAPROptionVector *ov2) > +{ > + g_assert(ov); > + g_assert(ov1); > + g_assert(ov2); > + > + bitmap_and(ov->bitmap, ov1->bitmap, ov2->bitmap, OV_MAXBITS); > +} > + > +/* returns true if options bits were removed, false otherwise */ > +bool spapr_ovec_diff(sPAPROptionVector *ov, > + sPAPROptionVector *ov_old, > + sPAPROptionVector *ov_new) > +{ > + unsigned long *change_mask = bitmap_new(OV_MAXBITS); > + unsigned long *removed_bits = bitmap_new(OV_MAXBITS); > + bool bits_were_removed = false; > + > + g_assert(ov); > + g_assert(ov_old); > + g_assert(ov_new); > + > + bitmap_xor(change_mask, ov_old->bitmap, ov_new->bitmap, OV_MAXBITS); > + bitmap_and(ov->bitmap, ov_new->bitmap, change_mask, OV_MAXBITS); > + bitmap_and(removed_bits, ov_old->bitmap, change_mask, OV_MAXBITS); > + > + if (!bitmap_empty(removed_bits, OV_MAXBITS)) { > + bits_were_removed = true; > + } > + > + g_free(change_mask); > + g_free(removed_bits); > + > + return bits_were_removed; > +} > + > +void spapr_ovec_cleanup(sPAPROptionVector *ov) > +{ > + if (ov) { > + g_free(ov->bitmap); > + g_free(ov); > + } > +} > + > +void spapr_ovec_set(sPAPROptionVector *ov, long bitnr) > +{ > + g_assert(ov); > + g_assert_cmpint(bitnr, <, OV_MAXBITS); > + > + set_bit(bitnr, ov->bitmap); > +} > + > +void spapr_ovec_clear(sPAPROptionVector *ov, long bitnr) > +{ > + g_assert(ov); > + g_assert_cmpint(bitnr, <, OV_MAXBITS); > + > + clear_bit(bitnr, ov->bitmap); > +} > + > +bool spapr_ovec_test(sPAPROptionVector *ov, long bitnr) > +{ > + g_assert(ov); > + g_assert_cmpint(bitnr, <, OV_MAXBITS); > + > + return test_bit(bitnr, ov->bitmap) ? true : false; > +} > + > +static void guest_byte_to_bitmap(uint8_t entry, unsigned long *bitmap, > + long bitmap_offset) > +{ > + int i; > + > + for (i = 0; i < BITS_PER_BYTE; i++) { > + if (entry & (1 << (BITS_PER_BYTE - 1 - i))) { > + bitmap_set(bitmap, bitmap_offset + i, 1); > + } > + } > +} > + > +static uint8_t guest_byte_from_bitmap(unsigned long *bitmap, long > bitmap_offset) > +{ > + uint8_t entry = 0; > + int i; > + > + for (i = 0; i < BITS_PER_BYTE; i++) { > + if (test_bit(bitmap_offset + i, bitmap)) { > + entry |= (1 << (BITS_PER_BYTE - 1 - i)); > + } > + } > + > + return entry; > +} > + > +static target_ulong vector_addr(target_ulong table_addr, int vector) > +{ > + uint16_t vector_count, vector_len; > + int i; > + > + vector_count = ldub_phys(&address_space_memory, table_addr) + 1; > + if (vector > vector_count) { > + return 0; > + } > + table_addr++; /* skip nr option vectors */ > + > + for (i = 0; i < vector - 1; i++) { > + vector_len = ldub_phys(&address_space_memory, table_addr) + 2; > + table_addr += vector_len; > + } > + return table_addr; > +} > + > +sPAPROptionVector *spapr_ovec_parse_vector(target_ulong table_addr, int > vector) > +{ > + unsigned long *bitmap; > + target_ulong addr; > + uint16_t vector_len; > + int i; > + > + g_assert(table_addr); > + g_assert_cmpint(vector, >=, 1); /* vector numbering starts at 1 */ > + > + addr = vector_addr(table_addr, vector); > + if (!addr) { > + /* specified vector isn't present */ > + return NULL; > + } > + > + vector_len = ldub_phys(&address_space_memory, addr++) + 1; Here you use vector_len to be the number of bytes _not_ including the length byte, but in other places you use the same name including the length byte, which is a litle confusing. > + if (vector_len >= OV_MAXBYTES) { Do you mean >= here, or >? If so, what's wrong with vector_len == 256, I thought that was explicitly permitted in the encoding? If not, then there's no need for the test since a byte load + 1 can't possibly exceed 256 (you could have an assert if you want). > + error_report("guest option vector length %i exceeds max of %i", > + vector_len, OV_MAXBYTES); > + } > + bitmap = bitmap_new(OV_MAXBITS); > + > + for (i = 0; i < vector_len; i++) { > + uint8_t entry = ldub_phys(&address_space_memory, addr + i); > + if (entry) { > + DPRINTFN("read guest vector %2d, byte %3d / %3d: 0x%.2x", > + vector, i + 1, vector_len, entry); > + guest_byte_to_bitmap(entry, bitmap, i * BITS_PER_BYTE); > + } > + } > + > + return spapr_ovec_from_bitmap(bitmap); This is the only caller of spapr_ovec_from_bitmap(). You could equally well just use ovec_new() here and reach in to populate the bitmap. Means you don't need to expose spapr_ovec_from_bitmap() which is only safe if the supplied bitmap is the right size. > +} > + > +int spapr_ovec_populate_dt(void *fdt, int fdt_offset, > + sPAPROptionVector *ov, const char *name) > +{ > + uint8_t vec[OV_MAXBYTES + 1]; > + uint16_t vec_len; > + unsigned long lastbit; > + int i; > + > + g_assert(ov); > + > + lastbit = MIN(find_last_bit(ov->bitmap, OV_MAXBITS), OV_MAXBITS - 1); > + vec_len = lastbit / BITS_PER_BYTE + 2; If no bits are set at all, find_last_bit() will return 2048, which means you'll include a max size vector when you actually want a minimum size vector. > + g_assert_cmpint(vec_len - 2, <=, UINT8_MAX); > + vec[0] = vec_len - 2; /* guest expects length encoded as n - 2 */ > + > + for (i = 1; i < vec_len; i++) { > + vec[i] = guest_byte_from_bitmap(ov->bitmap, (i - 1) * BITS_PER_BYTE); > + if (vec[i]) { > + DPRINTFN("encoding guest vector byte %3d / %3d: 0x%.2x", > + i, vec_len, vec[i]); > + } > + } > + > + return fdt_setprop(fdt, fdt_offset, name, vec, vec_len); > +} > diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h > new file mode 100644 > index 0000000..fba2d98 > --- /dev/null > +++ b/include/hw/ppc/spapr_ovec.h > @@ -0,0 +1,62 @@ > +/* > + * QEMU SPAPR Option/Architecture Vector Definitions > + * > + * Each architecture option is organized/documented by the following > + * in LoPAPR 1.1, Table 244: > + * > + * <vector number>: the bit-vector in which the option is located > + * <vector byte>: the byte offset of the vector entry > + * <vector bit>: the bit offset within the vector entry > + * > + * where each vector entry can be one or more bytes. > + * > + * Firmware expects a somewhat literal encoding of this bit-vector > + * structure, where each entry is stored in little-endian so that the > + * byte ordering reflects that of the documentation, but where each bit > + * offset is from "left-to-right" in the traditional representation of > + * a byte value where the MSB is the left-most bit. Thus, each > + * individual byte encodes the option bits in reverse order of the > + * documented bit. > + * > + * These definitions/helpers attempt to abstract away this internal > + * representation so that we can define/set/test for individual option > + * bits using only the documented values. This is done mainly by relying > + * on a bitmap to approximate the documented "bit-vector" structure and > + * handling conversations to-from the internal representation under the > + * covers. > + * > + * Copyright IBM Corp. 2016 > + * > + * Authors: > + * Michael Roth <mdr...@linux.vnet.ibm.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + */ > +#if !defined(__HW_SPAPR_OPTION_VECTORS_H__) > +#define __HW_SPAPR_OPTION_VECTORS_H__ > + > +#include "cpu.h" > + > +typedef struct sPAPROptionVector sPAPROptionVector; > + > +#define OV_BIT(byte, bit) ((byte - 1) * BITS_PER_BYTE + bit) > + > +/* interfaces */ > +sPAPROptionVector *spapr_ovec_new(void); > +sPAPROptionVector *spapr_ovec_clone(sPAPROptionVector *ov_orig); > +void spapr_ovec_intersect(sPAPROptionVector *ov, > + sPAPROptionVector *ov1, > + sPAPROptionVector *ov2); > +bool spapr_ovec_diff(sPAPROptionVector *ov, > + sPAPROptionVector *ov_old, > + sPAPROptionVector *ov_new); > +void spapr_ovec_cleanup(sPAPROptionVector *ov); > +void spapr_ovec_set(sPAPROptionVector *ov, long bitnr); > +void spapr_ovec_clear(sPAPROptionVector *ov, long bitnr); > +bool spapr_ovec_test(sPAPROptionVector *ov, long bitnr); > +sPAPROptionVector *spapr_ovec_parse_vector(target_ulong table_addr, int > vector); > +int spapr_ovec_populate_dt(void *fdt, int fdt_offset, > + sPAPROptionVector *ov, const char *name); > + > +#endif /* !defined (__HW_SPAPR_OPTION_VECTORS_H__) */ -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
