Re: [Qemu-devel] [PATCH 2/4] pci: Add generic PCIe host bridge

[Alexander Graf]

On 12.01.15 18:36, alvise rigo wrote:
> Hi Alexander,
> 
> Just a comment below.
> 
> On Tue, Jan 6, 2015 at 5:03 PM, Alexander Graf <address@hidden> wrote:
>> With simple exposure of MMFG, ioport window, mmio window and an IRQ line we
>> can successfully create a workable PCIe host bridge that can be mapped 
>> anywhere
>> and only needs to get described to the OS using whatever means it likes.
>>
>> This patch implements such a "generic" host bridge. It only supports a single
>> legacy IRQ line so far. MSIs need to be handled external to the host bridge.
>>
>> This device is particularly useful for the "pci-host-ecam-generic" driver in
>> Linux.
>>
>> Signed-off-by: Alexander Graf <address@hidden>
>> ---
>>  hw/pci-host/Makefile.objs  |   1 +
>>  hw/pci-host/gpex.c         | 156 
>> +++++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/pci-host/gpex.h |  56 ++++++++++++++++
>>  3 files changed, 213 insertions(+)
>>  create mode 100644 hw/pci-host/gpex.c
>>  create mode 100644 include/hw/pci-host/gpex.h
>>
>> diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs
>> index bb65f9c..45f1f0e 100644
>> --- a/hw/pci-host/Makefile.objs
>> +++ b/hw/pci-host/Makefile.objs
>> @@ -15,3 +15,4 @@ common-obj-$(CONFIG_PCI_APB) += apb.o
>>  common-obj-$(CONFIG_FULONG) += bonito.o
>>  common-obj-$(CONFIG_PCI_PIIX) += piix.o
>>  common-obj-$(CONFIG_PCI_Q35) += q35.o
>> +common-obj-$(CONFIG_PCI_GENERIC) += gpex.o
>> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
>> new file mode 100644
>> index 0000000..bd62a3c
>> --- /dev/null
>> +++ b/hw/pci-host/gpex.c
>> @@ -0,0 +1,156 @@
>> +/*
>> + * QEMU Generic PCI Express Bridge Emulation
>> + *
>> + * Copyright (C) 2015 Alexander Graf <address@hidden>
>> + *
>> + * Code loosely based on q35.c.
>> + *
>> + * 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 "hw/hw.h"
>> +#include "hw/pci-host/gpex.h"
>> +
>> +/****************************************************************************
>> + * GPEX host
>> + */
>> +
>> +static void gpex_set_irq(void *opaque, int irq_num, int level)
>> +{
>> +    GPEXHost *s = opaque;
>> +
>> +    qemu_set_irq(s->irq, level);
>> +}
>> +
>> +static int gpex_map_irq(PCIDevice *pci_dev, int irq_num)
>> +{
>> +    /* We only support one IRQ line so far */
>> +    return 0;
>> +}
> 
> Regarding the request from Claudio to have one system interrupt for
> each PCI device, we could address this by swizzling four (or more)
> system interrupts for all the PCI devices. In this case a slightly
> different interrupt-map and interrupt-map-mask is required as well as
> a new map_irq callback (the legacy pci_swizzle_map_irq_fn is fine for
> 4 IRQs).
> This of course would work as far as we have less PCI devices than the
> number of swizzled IRQs.

I'd prefer to keep things as easy as we humanly can for now. Then add
MSI. And if we then realize that we still need 4 rather than 1 shared
interrupt lines we can still change it :).


Alex