[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addres
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses |
Date: |
Mon, 9 Sep 2013 14:40:29 +0300 |
On Mon, Sep 09, 2013 at 02:11:54PM +0300, Marcel Apfelbaum wrote:
> Created a MemoryRegion with negative priority that
> spans over all the pci address space.
> It "intercepts" the accesses to unassigned pci
> address space and will follow the pci spec:
> 1. returns -1 on read
> 2. does nothing on write
> 3. sets the RECEIVED MASTER ABORT bit in the STATUS register
> of the device that initiated the transaction
>
> Note: This implementation assumes that all the reads/writes to
> the pci address space are done by the cpu.
>
> Signed-off-by: Marcel Apfelbaum <address@hidden>
> ---
> Changes from v1:
> - "pci-unassigned-mem" MemoryRegion resides now in PCIBus and not on
> various Host Bridges
> - "pci-unassgined-mem" does not have a ".valid.accept" field and
> implements read write methods
>
> hw/pci/pci.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/pci/pci_bus.h | 1 +
> 2 files changed, 47 insertions(+)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d00682e..b6a8026 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -283,6 +283,43 @@ const char *pci_root_bus_path(PCIDevice *dev)
> return rootbus->qbus.name;
> }
>
> +static void unassigned_mem_access(PCIBus *bus)
> +{
> + /* FIXME assumption: memory access to the pci address
> + * space is always initiated by the host bridge
/* Always
* like
* this
*/
/* Not
* like this */
> + * (device 0 on the bus) */
Can't we pass the master device in?
We are still left with the assumption that
there's a single master, but at least
we get rid of the assumption that it's
always device 0 function 0.
> + PCIDevice *d = bus->devices[0];
> + if (!d) {
> + return;
> + }
> +
> + pci_word_test_and_set_mask(d->config + PCI_STATUS,
> + PCI_STATUS_REC_MASTER_ABORT);
Probably should check and set secondary status for
a bridge device.
> +}
> +
> +static uint64_t unassigned_mem_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + PCIBus *bus = opaque;
> + unassigned_mem_access(bus);
> +
> + return -1ULL;
> +}
> +
> +static void unassigned_mem_write(void *opaque, hwaddr addr, uint64_t val,
> + unsigned size)
> +{
> + PCIBus *bus = opaque;
> + unassigned_mem_access(bus);
> +}
> +
> +static const MemoryRegionOps unassigned_mem_ops = {
> + .read = unassigned_mem_read,
> + .write = unassigned_mem_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +#define UNASSIGNED_MEM_PRIORITY -1
> +
> static void pci_bus_init(PCIBus *bus, DeviceState *parent,
> const char *name,
> MemoryRegion *address_space_mem,
I would rename "unassigned" to "pci_master_Abort_" everywhere.
Call things by what they do not how they are used.
> @@ -294,6 +331,15 @@ static void pci_bus_init(PCIBus *bus, DeviceState
> *parent,
> bus->address_space_mem = address_space_mem;
> bus->address_space_io = address_space_io;
>
> +
> + memory_region_init_io(&bus->unassigned_mem, OBJECT(bus),
> + &unassigned_mem_ops, bus, "pci-unassigned",
> + memory_region_size(bus->address_space_mem));
> + memory_region_add_subregion_overlap(bus->address_space_mem,
> + bus->address_space_mem->addr,
> + &bus->unassigned_mem,
> + UNASSIGNED_MEM_PRIORITY);
> +
> /* host bridge */
> QLIST_INIT(&bus->child);
>
It seems safer to add an API for enabling this functionality.
Something like
pci_set_master_for_master_abort(PCIBus *, PCIDevice *);
> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
> index 9df1788..4cc25a3 100644
> --- a/include/hw/pci/pci_bus.h
> +++ b/include/hw/pci/pci_bus.h
> @@ -23,6 +23,7 @@ struct PCIBus {
> PCIDevice *parent_dev;
> MemoryRegion *address_space_mem;
> MemoryRegion *address_space_io;
> + MemoryRegion unassigned_mem;
>
> QLIST_HEAD(, PCIBus) child; /* this will be replaced by qdev later */
> QLIST_ENTRY(PCIBus) sibling;/* this will be replaced by qdev later */
> --
> 1.8.3.1
- [Qemu-devel] [PATCH RFC v2 0/3] pci: complete master abort protocol, Marcel Apfelbaum, 2013/09/09
- [Qemu-devel] [PATCH RFC v2 1/2] memory: allow MemoryRegion's priority field to accept negative values, Marcel Apfelbaum, 2013/09/09
- [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Marcel Apfelbaum, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses,
Michael S. Tsirkin <=
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Marcel Apfelbaum, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Michael S. Tsirkin, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Marcel Apfelbaum, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Peter Maydell, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Michael S. Tsirkin, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Peter Maydell, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Marcel Apfelbaum, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Peter Maydell, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Marcel Apfelbaum, 2013/09/09
- Re: [Qemu-devel] [PATCH RFC v2 2/2] hw/pci: handle unassigned pci addresses, Peter Maydell, 2013/09/09