Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/6] pseries: Implement PAPR NVRAM

[Alexander Graf]

On 26.09.2012, at 14:55, Alexander Graf wrote:

> 
> On 26.09.2012, at 05:12, David Gibson wrote:
> 
>> The PAPR specification requires a certain amount of NVRAM, accessed via
>> RTAS, which we don't currently implement in qemu.  This patch addresses
>> this deficiency, implementing the NVRAM as a VIO device, with some glue to
>> instantiate it automatically based on a machine option.
>> 
>> The machine option specifies a drive id, which is used to back the NVRAM,
>> making it persistent.  If nothing is specified, the driver instead simply
>> allocates space for the NVRAM, which will not be persistent
>> 
>> Signed-off-by: David Gibson <address@hidden>
>> ---
>> hw/ppc/Makefile.objs |    1 +
>> hw/spapr.c           |    3 +
>> hw/spapr.h           |    3 +
>> hw/spapr_nvram.c     |  225 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>> qemu-config.c        |    4 +
>> 5 files changed, 236 insertions(+)
>> create mode 100644 hw/spapr_nvram.c
>> 
>> diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
>> index 951e407..91cbe8c 100644
>> --- a/hw/ppc/Makefile.objs
>> +++ b/hw/ppc/Makefile.objs
>> @@ -11,6 +11,7 @@ obj-y += ppc_newworld.o
>> obj-$(CONFIG_PSERIES) += spapr.o spapr_hcall.o spapr_rtas.o spapr_vio.o
>> obj-$(CONFIG_PSERIES) += xics.o spapr_vty.o spapr_llan.o spapr_vscsi.o
>> obj-$(CONFIG_PSERIES) += spapr_pci.o pci-hotplug.o spapr_iommu.o
>> +obj-$(CONFIG_PSERIES) += spapr_nvram.o
>> # PowerPC 4xx boards
>> obj-y += ppc4xx_devs.o ppc4xx_pci.o ppc405_uc.o ppc405_boards.o
>> obj-y += ppc440_bamboo.o
>> diff --git a/hw/spapr.c b/hw/spapr.c
>> index a8bd3c1..079825a 100644
>> --- a/hw/spapr.c
>> +++ b/hw/spapr.c
>> @@ -804,6 +804,9 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>>        }
>>    }
>> 
>> +    /* We always have at least the nvram device on VIO */
>> +    spapr_create_nvram(spapr);
>> +
>>    /* Set up PCI */
>>    spapr_pci_rtas_init();
>> 
>> diff --git a/hw/spapr.h b/hw/spapr.h
>> index e984e3f..d9c3b4a 100644
>> --- a/hw/spapr.h
>> +++ b/hw/spapr.h
>> @@ -6,11 +6,13 @@
>> 
>> struct VIOsPAPRBus;
>> struct sPAPRPHBState;
>> +struct sPAPRNVRAM;
>> struct icp_state;
>> 
>> typedef struct sPAPREnvironment {
>>    struct VIOsPAPRBus *vio_bus;
>>    QLIST_HEAD(, sPAPRPHBState) phbs;
>> +    struct sPAPRNVRAM *nvram;
>>    struct icp_state *icp;
>> 
>>    target_phys_addr_t ram_limit;
>> @@ -336,6 +338,7 @@ typedef struct sPAPRTCE {
>> #define SPAPR_PCI_BASE_LIOBN    0x80000000
>> 
>> void spapr_iommu_init(void);
>> +void spapr_create_nvram(sPAPREnvironment *spapr);
>> DMAContext *spapr_tce_new_dma_context(uint32_t liobn, size_t window_size);
>> void spapr_tce_free(DMAContext *dma);
>> void spapr_tce_reset(DMAContext *dma);
>> diff --git a/hw/spapr_nvram.c b/hw/spapr_nvram.c
>> new file mode 100644
>> index 0000000..8cd8a53
>> --- /dev/null
>> +++ b/hw/spapr_nvram.c
>> @@ -0,0 +1,225 @@
>> +/*
>> + * QEMU sPAPR NVRAM emulation
>> + *
>> + * Copyright (C) 2012 David Gibson, IBM Corporation.
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a 
>> copy
>> + * of this software and associated documentation files (the "Software"), to 
>> deal
>> + * in the Software without restriction, including without limitation the 
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included 
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
>> FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +#include <sys/mman.h>
>> +#include <libfdt.h>
>> +
>> +#include "device_tree.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/spapr.h"
>> +#include "hw/spapr_vio.h"
>> +
>> +typedef struct sPAPRNVRAM {
>> +    VIOsPAPRDevice sdev;
>> +    uint32_t size;
>> +    uint8_t *buf;
>> +    BlockDriverState *drive;
>> +} sPAPRNVRAM;
>> +
>> +#define MIN_NVRAM_SIZE 8192
>> +#define DEFAULT_NVRAM_SIZE 16384
>> +#define MAX_NVRAM_SIZE (UINT16_MAX * 16)
>> +
>> +static void rtas_nvram_fetch(sPAPREnvironment *spapr,
>> +                             uint32_t token, uint32_t nargs,
>> +                             target_ulong args,
>> +                             uint32_t nret, target_ulong rets)
>> +{
>> +    sPAPRNVRAM *nvram = spapr->nvram;
>> +    target_phys_addr_t offset, buffer, len;
>> +    int alen;
>> +    void *membuf;
>> +
>> +    if ((nargs != 3) || (nret != 2)) {
>> +        rtas_st(rets, 0, -3);
>> +        return;
>> +    }
>> +
>> +    if (!nvram) {
>> +        rtas_st(rets, 0, -1);
>> +        rtas_st(rets, 1, 0);
>> +        return;
>> +    }
>> +
>> +    offset = rtas_ld(args, 0);
>> +    buffer = rtas_ld(args, 1);
>> +    len = rtas_ld(args, 2);
>> +
>> +    if (((offset + len) < offset)
>> +        || ((offset + len) > nvram->size)) {
>> +        rtas_st(rets, 0, -3);
>> +        rtas_st(rets, 1, 0);
>> +        return;
>> +    }
>> +
>> +    membuf = cpu_physical_memory_map(buffer, &len, 1);
>> +    if (nvram->drive) {
>> +        alen = bdrv_pread(nvram->drive, offset, membuf, len);
>> +    } else {
>> +        assert(nvram->buf);
>> +
>> +        memcpy(membuf, nvram->buf + offset, len);
>> +        alen = len;
>> +    }
>> +    cpu_physical_memory_unmap(membuf, len, 1, len);
>> +
>> +    rtas_st(rets, 0, (alen < len) ? -1 : 0);
>> +    rtas_st(rets, 1, (alen < 0) ? 0 : alen);
>> +}
>> +
>> +static void rtas_nvram_store(sPAPREnvironment *spapr,
>> +                             uint32_t token, uint32_t nargs,
>> +                             target_ulong args,
>> +                             uint32_t nret, target_ulong rets)
>> +{
>> +    sPAPRNVRAM *nvram = spapr->nvram;
>> +    target_phys_addr_t offset, buffer, len;
>> +    int alen;
>> +    void *membuf;
>> +
>> +    if ((nargs != 3) || (nret != 2)) {
>> +        rtas_st(rets, 0, -3);
>> +        return;
>> +    }
>> +
>> +    if (!nvram) {
>> +        rtas_st(rets, 0, -1);
>> +        return;
>> +    }
>> +
>> +    offset = rtas_ld(args, 0);
>> +    buffer = rtas_ld(args, 1);
>> +    len = rtas_ld(args, 2);
>> +
>> +    if (((offset + len) < offset)
>> +        || ((offset + len) > nvram->size)) {
>> +        rtas_st(rets, 0, -3);
>> +        return;
>> +    }
>> +
>> +    membuf = cpu_physical_memory_map(buffer, &len, 0);
>> +    if (nvram->drive) {
>> +        alen = bdrv_pwrite(nvram->drive, offset, membuf, len);
>> +    } else {
>> +        assert(nvram->buf);
>> +
>> +        memcpy(nvram->buf + offset, membuf, len);
>> +        alen = len;
>> +    }
>> +    cpu_physical_memory_unmap(membuf, len, 0, len);
>> +
>> +    rtas_st(rets, 0, (alen < len) ? -1 : 0);
>> +    rtas_st(rets, 1, (alen < 0) ? 0 : alen);
>> +}
>> +
>> +static int spapr_nvram_init(VIOsPAPRDevice *dev)
>> +{
>> +    sPAPRNVRAM *nvram = (sPAPRNVRAM *)dev;
>> +
>> +    if (nvram->drive) {
>> +        nvram->size = bdrv_getlength(nvram->drive);
>> +    } else {
>> +        nvram->size = DEFAULT_NVRAM_SIZE;
>> +        nvram->buf = g_malloc0(nvram->size);
>> +    }
>> +
>> +    if ((nvram->size < MIN_NVRAM_SIZE) || (nvram->size > MAX_NVRAM_SIZE)) {
>> +        fprintf(stderr, "spapr-nvram must be between %d and %d bytes in 
>> size\n",
>> +                MIN_NVRAM_SIZE, MAX_NVRAM_SIZE);
>> +        return -1;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int spapr_nvram_devnode(VIOsPAPRDevice *dev, void *fdt, int node_off)
>> +{
>> +    sPAPRNVRAM *nvram = (sPAPRNVRAM *)dev;
>> +
>> +    return fdt_setprop_cell(fdt, node_off, "#bytes", nvram->size);
>> +}
>> +
>> +static Property spapr_nvram_properties[] = {
>> +    DEFINE_SPAPR_PROPERTIES(sPAPRNVRAM, sdev),
>> +    DEFINE_PROP_DRIVE("drive", sPAPRNVRAM, drive),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void spapr_nvram_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    VIOsPAPRDeviceClass *k = VIO_SPAPR_DEVICE_CLASS(klass);
>> +
>> +    k->init = spapr_nvram_init;
>> +    k->devnode = spapr_nvram_devnode;
>> +    k->dt_name = "nvram";
>> +    k->dt_type = "nvram";
>> +    k->dt_compatible = "qemu,spapr-nvram";
>> +    dc->props = spapr_nvram_properties;
>> +}
>> +
>> +static const TypeInfo spapr_nvram_type_info = {
>> +    .name          = "spapr-nvram",
>> +    .parent        = TYPE_VIO_SPAPR_DEVICE,
>> +    .instance_size = sizeof(sPAPRNVRAM),
>> +    .class_init    = spapr_nvram_class_init,
>> +};
>> +
>> +static void spapr_nvram_register_types(void)
>> +{
>> +    type_register_static(&spapr_nvram_type_info);
>> +}
>> +
>> +type_init(spapr_nvram_register_types)
>> +
>> +void spapr_create_nvram(sPAPREnvironment *spapr)
> 
> Could you please move this function into spapr.c?

Reading this again I'm missing the explanation :).

I do understand the intention of having device initialization code for nvram in 
the nvram source code.

But semantically, it's the spapr machine creating the device. It should not 
have knowledge of any internals of the device, so the code can live anywhere 
anyways. However, by moving it into spapr.c, we can make it local to the spapr 
machine. If someone else wants to -device an spapr-nvram, they can do so (it 
will fail because there are no hypercall registrations there, but that's a 
different matter). But all the machine semantics of how to read the 
machine_opts live in the machine file then :).

I hope that explains things a bit better. I didn't realize how short my reply 
was - sorry :).


Alex