Ping, anyone?
On 07/23/2013 12:54 PM, Alexey Kardashevskiy wrote:
> Michael, could you please review the patch as it is about PCI? Thanks!
>
>
> On 07/12/2013 05:38 PM, Alexey Kardashevskiy wrote:
>> On the sPAPR platform a guest allocates MSI/MSIX vectors via RTAS
>> hypercalls which return global IRQ numbers to a guest so it only
>> operates with those and never touches MSIMessage.
>>
>> Therefore MSIMessage handling is completely hidden in QEMU.
>>
>> Previously every sPAPR PCI host bridge implemented its own MSI window
>> to catch msi_notify()/msix_notify() calls from QEMU devices (virtio-pci
>> or vfio) and route them to the guest via qemu_pulse_irq().
>> MSIMessage used to be encoded as:
>> .addr - address within the PHB MSI window;
>> .data - the device index on PHB plus vector number.
>> The MSI MR write function translated this MSIMessage to a global IRQ
>> number and called qemu_pulse_irq().
>>
>> However the total number of IRQs is not really big (at the moment it is
>> 1024 IRQs starting from 4096) and even 16bit data field of MSIMessage
>> seems to be enough to store an IRQ number there.
>>
>> This simplifies MSI handling in sPAPR PHB. Specifically, this does:
>> 1. remove a MSI window from a PHB;
>> 2. add a single memory region for all MSIs to sPAPREnvironment
>> and spapr_pci_msi_init() to initialize it;
>> 3. encode MSIMessage as:
>> * .addr - a fixed address of SPAPR_PCI_MSI_WINDOW==0x40000000000ULL;
>> * .data as an IRQ number.
>> 4. change IRQ allocator to align first IRQ number in a block for MSI.
>> MSI uses lower bits to specify the vector number so the first IRQ has to
>> be aligned. MSIX does not need any special allocator though.
>>
>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
>> ---
>> hw/ppc/spapr.c | 29 ++++++++++++++++++---
>> hw/ppc/spapr_pci.c | 61
>> ++++++++++++++++++++-------------------------
>> include/hw/pci-host/spapr.h | 8 +++---
>> include/hw/ppc/spapr.h | 4 ++-
>> 4 files changed, 60 insertions(+), 42 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 671bbd9..6b21191 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -88,6 +88,9 @@ int spapr_allocate_irq(int hint, bool lsi)
>>
>> if (hint) {
>> irq = hint;
>> + if (hint >= spapr->next_irq) {
>> + spapr->next_irq = hint + 1;
>> + }
>> /* FIXME: we should probably check for collisions somehow */
>> } else {
>> irq = spapr->next_irq++;
>> @@ -103,22 +106,39 @@ int spapr_allocate_irq(int hint, bool lsi)
>> return irq;
>> }
>>
>> -/* Allocate block of consequtive IRQs, returns a number of the first */
>> -int spapr_allocate_irq_block(int num, bool lsi)
>> +/*
>> + * Allocate block of consequtive IRQs, returns a number of the first.
>> + * If msi==true, aligns the first IRQ number to num.
>> + */
>> +int spapr_allocate_irq_block(int num, bool lsi, bool msi)
>> {
>> int first = -1;
>> - int i;
>> + int i, hint = 0;
>> +
>> + /*
>> + * MSIMesage::data is used for storing VIRQ so
>> + * it has to be aligned to num to support multiple
>> + * MSI vectors. MSI-X is not affected by this.
>> + * The hint is used for the first IRQ, the rest should
>> + * be allocated continously.
>> + */
>> + if (msi) {
>> + assert((num == 1) || (num == 2) || (num == 4) ||
>> + (num == 8) || (num == 16) || (num == 32));
>> + hint = (spapr->next_irq + num - 1) & ~(num - 1);
>> + }
>>
>> for (i = 0; i < num; ++i) {
>> int irq;
>>
>> - irq = spapr_allocate_irq(0, lsi);
>> + irq = spapr_allocate_irq(hint, lsi);
>> if (!irq) {
>> return -1;
>> }
>>
>> if (0 == i) {
>> first = irq;
>> + hint = 0;
>> }
>>
>> /* If the above doesn't create a consecutive block then that's
>> @@ -1127,6 +1147,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>> spapr_create_nvram(spapr);
>>
>> /* Set up PCI */
>> + spapr_pci_msi_init(spapr, SPAPR_PCI_MSI_WINDOW);
>> spapr_pci_rtas_init();
>>
>> phb = spapr_create_phb(spapr, 0);
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index dfe4d04..c8ea777 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -258,11 +258,11 @@ static int spapr_msicfg_find(sPAPRPHBState *phb,
>> uint32_t config_addr,
>> * This is required for msi_notify()/msix_notify() which
>> * will write at the addresses via spapr_msi_write().
>> */
>> -static void spapr_msi_setmsg(PCIDevice *pdev, hwaddr addr,
>> - bool msix, unsigned req_num)
>> +static void spapr_msi_setmsg(PCIDevice *pdev, hwaddr addr, bool msix,
>> + unsigned first_irq, unsigned req_num)
>> {
>> unsigned i;
>> - MSIMessage msg = { .address = addr, .data = 0 };
>> + MSIMessage msg = { .address = addr, .data = first_irq };
>>
>> if (!msix) {
>> msi_set_message(pdev, msg);
>> @@ -270,8 +270,7 @@ static void spapr_msi_setmsg(PCIDevice *pdev, hwaddr
>> addr,
>> return;
>> }
>>
>> - for (i = 0; i < req_num; ++i) {
>> - msg.address = addr | (i << 2);
>> + for (i = 0; i < req_num; ++i, ++msg.data) {
>> msix_set_message(pdev, i, msg);
>> trace_spapr_pci_msi_setup(pdev->name, i, msg.address);
>> }
>> @@ -351,7 +350,8 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu,
>> sPAPREnvironment *spapr,
>>
>> /* There is no cached config, allocate MSIs */
>> if (!phb->msi_table[ndev].nvec) {
>> - irq = spapr_allocate_irq_block(req_num, false);
>> + irq = spapr_allocate_irq_block(req_num, false,
>> + ret_intr_type == RTAS_TYPE_MSI);
>> if (irq < 0) {
>> fprintf(stderr, "Cannot allocate MSIs for device#%d", ndev);
>> rtas_st(rets, 0, -1); /* Hardware error */
>> @@ -363,8 +363,8 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu,
>> sPAPREnvironment *spapr,
>> }
>>
>> /* Setup MSI/MSIX vectors in the device (via cfgspace or MSIX BAR) */
>> - spapr_msi_setmsg(pdev, phb->msi_win_addr | (ndev << 16),
>> - ret_intr_type == RTAS_TYPE_MSIX, req_num);
>> + spapr_msi_setmsg(pdev, spapr->msi_win_addr, ret_intr_type ==
>> RTAS_TYPE_MSIX,
>> + phb->msi_table[ndev].irq, req_num);
>>
>> rtas_st(rets, 0, 0);
>> rtas_st(rets, 1, req_num);
>> @@ -487,10 +487,7 @@ static const MemoryRegionOps spapr_io_ops = {
>> static void spapr_msi_write(void *opaque, hwaddr addr,
>> uint64_t data, unsigned size)
>> {
>> - sPAPRPHBState *phb = opaque;
>> - int ndev = addr >> 16;
>> - int vec = ((addr & 0xFFFF) >> 2) | data;
>> - uint32_t irq = phb->msi_table[ndev].irq + vec;
>> + uint32_t irq = data;
>>
>> trace_spapr_pci_msi_write(addr, data, irq);
>>
>> @@ -504,6 +501,23 @@ static const MemoryRegionOps spapr_msi_ops = {
>> .endianness = DEVICE_LITTLE_ENDIAN
>> };
>>
>> +void spapr_pci_msi_init(sPAPREnvironment *spapr, hwaddr addr)
>> +{
>> + /*
>> + * As MSI/MSIX interrupts trigger by writing at MSI/MSIX vectors,
>> + * we need to allocate some memory to catch those writes coming
>> + * from msi_notify()/msix_notify().
>> + * As MSIMessage:addr is going to be the same and MSIMessage:data
>> + * is going to be a VIRQ number, 4 bytes of the MSI MR will only
>> + * be used.
>> + */
>> + spapr->msi_win_addr = addr;
>> + memory_region_init_io(&spapr->msiwindow, NULL, &spapr_msi_ops, spapr,
>> + "msi", getpagesize());
>> + memory_region_add_subregion(get_system_memory(), spapr->msi_win_addr,
>> + &spapr->msiwindow);
>> +}
>> +
>> /*
>> * PHB PCI device
>> */
>> @@ -528,8 +542,7 @@ static int spapr_phb_init(SysBusDevice *s)
>>
>> if ((sphb->buid != -1) || (sphb->dma_liobn != -1)
>> || (sphb->mem_win_addr != -1)
>> - || (sphb->io_win_addr != -1)
>> - || (sphb->msi_win_addr != -1)) {
>> + || (sphb->io_win_addr != -1)) {
>> fprintf(stderr, "Either \"index\" or other parameters must"
>> " be specified for PAPR PHB, not both\n");
>> return -1;
>> @@ -542,7 +555,6 @@ static int spapr_phb_init(SysBusDevice *s)
>> + sphb->index * SPAPR_PCI_WINDOW_SPACING;
>> sphb->mem_win_addr = windows_base + SPAPR_PCI_MMIO_WIN_OFF;
>> sphb->io_win_addr = windows_base + SPAPR_PCI_IO_WIN_OFF;
>> - sphb->msi_win_addr = windows_base + SPAPR_PCI_MSI_WIN_OFF;
>> }
>>
>> if (sphb->buid == -1) {
>> @@ -565,11 +577,6 @@ static int spapr_phb_init(SysBusDevice *s)
>> return -1;
>> }
>>
>> - if (sphb->msi_win_addr == -1) {
>> - fprintf(stderr, "MSI window address not specified for PHB\n");
>> - return -1;
>> - }
>> -
>> if (find_phb(spapr, sphb->buid)) {
>> fprintf(stderr, "PCI host bridges must have unique BUIDs\n");
>> return -1;
>> @@ -609,18 +616,6 @@ static int spapr_phb_init(SysBusDevice *s)
>> namebuf, SPAPR_PCI_IO_WIN_SIZE);
>> memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
>> &sphb->iowindow);
>> -
>> - /* As MSI/MSIX interrupts trigger by writing at MSI/MSIX vectors,
>> - * we need to allocate some memory to catch those writes coming
>> - * from msi_notify()/msix_notify() */
>> - if (msi_supported) {
>> - sprintf(namebuf, "%s.msi", sphb->dtbusname);
>> - memory_region_init_io(&sphb->msiwindow, OBJECT(sphb),
>> &spapr_msi_ops, sphb,
>> - namebuf, SPAPR_MSIX_MAX_DEVS * 0x10000);
>> - memory_region_add_subregion(get_system_memory(), sphb->msi_win_addr,
>> - &sphb->msiwindow);
>> - }
>> -
>> /*
>> * Selecting a busname is more complex than you'd think, due to
>> * interacting constraints. If the user has specified an id
>> @@ -695,7 +690,6 @@ static Property spapr_phb_properties[] = {
>> DEFINE_PROP_HEX64("io_win_addr", sPAPRPHBState, io_win_addr, -1),
>> DEFINE_PROP_HEX64("io_win_size", sPAPRPHBState, io_win_size,
>> SPAPR_PCI_IO_WIN_SIZE),
>> - DEFINE_PROP_HEX64("msi_win_addr", sPAPRPHBState, msi_win_addr, -1),
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>> @@ -737,7 +731,6 @@ static const VMStateDescription vmstate_spapr_pci = {
>> VMSTATE_UINT64_EQUAL(mem_win_size, sPAPRPHBState),
>> VMSTATE_UINT64_EQUAL(io_win_addr, sPAPRPHBState),
>> VMSTATE_UINT64_EQUAL(io_win_size, sPAPRPHBState),
>> - VMSTATE_UINT64_EQUAL(msi_win_addr, sPAPRPHBState),
>> VMSTATE_STRUCT_ARRAY(lsi_table, sPAPRPHBState, PCI_NUM_PINS, 0,
>> vmstate_spapr_pci_lsi, struct spapr_pci_lsi),
>> VMSTATE_STRUCT_ARRAY(msi_table, sPAPRPHBState, SPAPR_MSIX_MAX_DEVS,
>> 0,
>> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
>> index 93f9511..970b4a9 100644
>> --- a/include/hw/pci-host/spapr.h
>> +++ b/include/hw/pci-host/spapr.h
>> @@ -43,8 +43,7 @@ typedef struct sPAPRPHBState {
>>
>> MemoryRegion memspace, iospace;
>> hwaddr mem_win_addr, mem_win_size, io_win_addr, io_win_size;
>> - hwaddr msi_win_addr;
>> - MemoryRegion memwindow, iowindow, msiwindow;
>> + MemoryRegion memwindow, iowindow;
>>
>> uint32_t dma_liobn;
>> uint64_t dma_window_start;
>> @@ -73,7 +72,8 @@ typedef struct sPAPRPHBState {
>> #define SPAPR_PCI_MMIO_WIN_SIZE 0x20000000
>> #define SPAPR_PCI_IO_WIN_OFF 0x80000000
>> #define SPAPR_PCI_IO_WIN_SIZE 0x10000
>> -#define SPAPR_PCI_MSI_WIN_OFF 0x90000000
>> +
>> +#define SPAPR_PCI_MSI_WINDOW 0x40000000000ULL
>>
>> #define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL
>>
>> @@ -88,6 +88,8 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>> uint32_t xics_phandle,
>> void *fdt);
>>
>> +void spapr_pci_msi_init(sPAPREnvironment *spapr, hwaddr addr);
>> +
>> void spapr_pci_rtas_init(void);
>>
>> #endif /* __HW_SPAPR_PCI_H__ */
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index 7aa9a3d..aed02e1 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -14,6 +14,8 @@ struct icp_state;
>> typedef struct sPAPREnvironment {
>> struct VIOsPAPRBus *vio_bus;
>> QLIST_HEAD(, sPAPRPHBState) phbs;
>> + hwaddr msi_win_addr;
>> + MemoryRegion msiwindow;
>> struct sPAPRNVRAM *nvram;
>> struct icp_state *icp;
>>
>> @@ -303,7 +305,7 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu,
>> target_ulong opcode,
>> target_ulong *args);
>>
>> int spapr_allocate_irq(int hint, bool lsi);
>> -int spapr_allocate_irq_block(int num, bool lsi);
>> +int spapr_allocate_irq_block(int num, bool lsi, bool msi);
>>
>> static inline int spapr_allocate_msi(int hint)
>> {
>>
>
>
--
Alexey
