All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>,
	Kevin O'Connor <kevin@koconnor.net>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v6 7/7] hw/pci-bridge: format SeaBIOS-compliant OFW device node for PXB
Date: Wed, 17 Jun 2015 16:03:09 +0200	[thread overview]
Message-ID: <20150617160253-mutt-send-email-mst@redhat.com> (raw)
In-Reply-To: <55817D86.8090808@redhat.com>

On Wed, Jun 17, 2015 at 04:00:38PM +0200, Laszlo Ersek wrote:
> On 06/17/15 15:57, Michael S. Tsirkin wrote:
> > On Wed, Jun 17, 2015 at 02:45:05PM +0200, Laszlo Ersek wrote:
> >> SeaBIOS expects OpenFirmware device paths in the "bootorder" fw_cfg file
> >> to follow the pattern
> >>
> >>   /pci-root@N/pci@i0cf8/...
> > 
> > It's kind of crazy, isn't it?
> 
> It's... idiosyncratic.

Yes - hoping for an answer from Kevin.

> > /pci@i0cf8/pci-root@N would make some sense: access rootN through cf8.
> > 
> > But if bios needs to keep this for compatibility, maybe
> > we have too, to. Kevin?
> 
> The only reason I'm doing this is because SeaBIOS expects precisely this
> OFW devpath fragment.
> 
> Thanks
> Laszlo
> 
> > 
> >>
> >> for devices that live behind an extra root bus. The extra root bus in
> >> question is the N'th among the extra root bridges. (In other words, N
> >> gives the position of the affected extra root bus relative to the other
> >> extra root buses, in bus_nr order.) N starts at 1, and is formatted in
> >> hex.
> >>
> >> The "pci@i0cf8" node text is hardcoded in SeaBIOS (see the macro
> >> FW_PCI_DOMAIN).
> >>
> >> Cc: Kevin O'Connor <kevin@koconnor.net>
> >> Cc: Marcel Apfelbaum <marcel@redhat.com>
> >> Cc: Michael S. Tsirkin <mst@redhat.com>
> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> >> Tested-by: Marcel Apfelbaum <marcel@redhat.com>
> >> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
> > 
> > This makes this a PC-specific device, which is pretty ugly.
> > You need to hardcode /pci-root@N here, but why not get
> > /pci@i0cf8 from the real root?
> > 
> >> ---
> >>
> >> Notes:
> >>     v6:
> >>     - no changes
> >>     
> >>     v5:
> >>     - constify parameter and local variables of pxb_host_ofw_unit_address(),
> >>       in accord with the previous patch
> >>     
> >>     v4:
> >>     - new in v4
> >>
> >>  hw/pci-bridge/pci_expander_bridge.c | 39 ++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 38 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> >> index 4398d98..37574ec 100644
> >> --- a/hw/pci-bridge/pci_expander_bridge.c
> >> +++ b/hw/pci-bridge/pci_expander_bridge.c
> >> @@ -42,6 +42,8 @@ typedef struct PXBDev {
> >>      uint16_t numa_node;
> >>  } PXBDev;
> >>  
> >> +static GList *pxb_dev_list;
> >> +
> >>  #define TYPE_PXB_HOST "pxb-host"
> >>  
> >>  static int pxb_bus_num(PCIBus *bus)
> >> @@ -88,12 +90,29 @@ static const char *pxb_host_root_bus_path(PCIHostState *host_bridge,
> >>      return bus->bus_path;
> >>  }
> >>  
> >> +static char *pxb_host_ofw_unit_address(const SysBusDevice *dev)
> >> +{
> >> +    const PCIHostState *host = PCI_HOST_BRIDGE(dev);
> >> +    const PCIBus *bus;
> >> +    const PXBDev *pxb;
> >> +    int position;
> >> +
> >> +    bus = host->bus;
> >> +    pxb = PXB_DEV(bus->parent_dev);
> >> +    position = g_list_index(pxb_dev_list, pxb);
> >> +    assert(position >= 0);
> >> +
> >> +    return g_strdup_printf("%x/pci@i0cf8", position + 1);
> > 
> > Please document that this reverse order is to match
> > existing bios behaviour.
> > 
> > 
> >> +}
> >> +
> >>  static void pxb_host_class_init(ObjectClass *class, void *data)
> >>  {
> >>      DeviceClass *dc = DEVICE_CLASS(class);
> >> +    SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(class);
> >>      PCIHostBridgeClass *hc = PCI_HOST_BRIDGE_CLASS(class);
> >>  
> >> -    dc->fw_name = "pci";
> >> +    dc->fw_name = "pci-root";
> >> +    sbc->explicit_ofw_unit_address = pxb_host_ofw_unit_address;
> >>      hc->root_bus_path = pxb_host_root_bus_path;
> >>  }
> >>  
> >> @@ -148,6 +167,15 @@ static int pxb_map_irq_fn(PCIDevice *pci_dev, int pin)
> >>      return pin - PCI_SLOT(pxb->devfn);
> >>  }
> >>  
> >> +static gint pxb_compare(gconstpointer a, gconstpointer b)
> >> +{
> >> +    const PXBDev *pxb_a = a, *pxb_b = b;
> >> +
> >> +    return pxb_a->bus_nr < pxb_b->bus_nr ? -1 :
> >> +           pxb_a->bus_nr > pxb_b->bus_nr ?  1 :
> >> +           0;
> >> +}
> >> +
> >>  static int pxb_dev_initfn(PCIDevice *dev)
> >>  {
> >>      PXBDev *pxb = PXB_DEV(dev);
> >> @@ -191,9 +219,17 @@ static int pxb_dev_initfn(PCIDevice *dev)
> >>                                 PCI_STATUS_66MHZ | PCI_STATUS_FAST_BACK);
> >>      pci_config_set_class(dev->config, PCI_CLASS_BRIDGE_HOST);
> >>  
> >> +    pxb_dev_list = g_list_insert_sorted(pxb_dev_list, pxb, pxb_compare);
> >>      return 0;
> >>  }
> >>  
> >> +static void pxb_dev_exitfn(PCIDevice *pci_dev)
> >> +{
> >> +    PXBDev *pxb = PXB_DEV(pci_dev);
> >> +
> >> +    pxb_dev_list = g_list_remove(pxb_dev_list, pxb);
> >> +}
> >> +
> >>  static Property pxb_dev_properties[] = {
> >>      /* Note: 0 is not a legal a PXB bus number. */
> >>      DEFINE_PROP_UINT8("bus_nr", PXBDev, bus_nr, 0),
> >> @@ -207,6 +243,7 @@ static void pxb_dev_class_init(ObjectClass *klass, void *data)
> >>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> >>  
> >>      k->init = pxb_dev_initfn;
> >> +    k->exit = pxb_dev_exitfn;
> >>      k->vendor_id = PCI_VENDOR_ID_REDHAT;
> >>      k->device_id = PCI_DEVICE_ID_REDHAT_PXB;
> >>      k->class_id = PCI_CLASS_BRIDGE_HOST;
> >> -- 
> >> 1.8.3.1

  reply	other threads:[~2015-06-17 14:03 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-17 12:44 [Qemu-devel] [PATCH v6 0/7] PXB changes Laszlo Ersek
2015-06-17 12:44 ` [Qemu-devel] [PATCH v6 1/7] migration: introduce VMSTATE_BUFFER_UNSAFE_INFO_TEST() Laszlo Ersek
2015-06-17 12:45 ` [Qemu-devel] [PATCH v6 2/7] hw/pci-bridge: expose _test parameter in SHPC_VMSTATE() Laszlo Ersek
2015-06-17 12:45 ` [Qemu-devel] [PATCH v6 3/7] hw/pci-bridge: introduce "hotplug" property Laszlo Ersek
2015-06-17 13:42   ` Michael S. Tsirkin
2015-06-17 13:55     ` Laszlo Ersek
2015-06-17 14:02       ` Michael S. Tsirkin
2015-06-17 14:15         ` Laszlo Ersek
2015-06-17 19:54           ` Marcel Apfelbaum
2015-06-18 13:47         ` Paolo Bonzini
2015-06-18 14:44           ` Michael S. Tsirkin
2015-06-17 12:45 ` [Qemu-devel] [PATCH v6 4/7] hw/pci-bridge: disable hotplug in PXB Laszlo Ersek
2015-06-17 13:45   ` Michael S. Tsirkin
2015-06-17 19:52     ` Marcel Apfelbaum
2015-06-17 12:45 ` [Qemu-devel] [PATCH v6 5/7] hw/core: rebase sysbus_get_fw_dev_path() to g_strdup_printf() Laszlo Ersek
2015-06-17 13:46   ` Michael S. Tsirkin
2015-06-17 13:56     ` Laszlo Ersek
2015-06-17 12:45 ` [Qemu-devel] [PATCH v6 6/7] hw/core: explicit OFW unit address callback for SysBusDeviceClass Laszlo Ersek
2015-06-17 12:45 ` [Qemu-devel] [PATCH v6 7/7] hw/pci-bridge: format SeaBIOS-compliant OFW device node for PXB Laszlo Ersek
2015-06-17 13:57   ` Michael S. Tsirkin
2015-06-17 14:00     ` Laszlo Ersek
2015-06-17 14:03       ` Michael S. Tsirkin [this message]
2015-06-17 14:18     ` Kevin O'Connor
2015-06-17 14:45       ` Laszlo Ersek
2015-06-17 15:05         ` Michael S. Tsirkin
2015-06-17 18:16           ` Laszlo Ersek
2015-06-17 18:54             ` Michael S. Tsirkin
2015-06-17 19:15               ` Laszlo Ersek
2015-06-17 19:28                 ` Kevin O'Connor
2015-06-17 19:32                   ` Michael S. Tsirkin
2015-06-17 19:44                     ` Laszlo Ersek
2015-06-17 21:50                       ` Michael S. Tsirkin
2015-06-18 13:22                         ` Laszlo Ersek
2015-06-18 13:40                           ` Michael S. Tsirkin
2015-06-18 15:42                             ` Laszlo Ersek
2015-06-17 19:09             ` Kevin O'Connor
2015-06-17 19:21   ` Michael S. Tsirkin
2015-06-17 19:35     ` Laszlo Ersek
2015-06-17 21:49       ` Michael S. Tsirkin
2015-06-18 13:18         ` Laszlo Ersek
2015-06-17 19:38     ` Kevin O'Connor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150617160253-mutt-send-email-mst@redhat.com \
    --to=mst@redhat.com \
    --cc=kevin@koconnor.net \
    --cc=lersek@redhat.com \
    --cc=marcel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.