You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/05/05 16:58:25 UTC

[GitHub] [incubator-nuttx] sonicyang opened a new pull request #979: PCI-E support with x86_64/qemu example

sonicyang opened a new pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979


   ## Summary
   
   This patch adds a PCI-E subsystem and implemented a backend on x86_64 platform.
   
   It also includes a driver for QEMU's pci-testdev for testing purpose.
   
   ## Impact
   
   Additional PCI-E feature will be provided.
   
   ## Testing
   
   By enabling 
    * CONFIG_PCIE 
    * CONFIG_QEMU_PCIE
    * CONFIG_VIRT_QEMU_PCI_TEST
   
   Booting x86_64 qemu build on QEMU/KVM with `-device pci-testdev` will have verbose message during OS initialization about PCI-E devices.
   
   The driver will be probed and read/print out the test support by qemu's pci-testdev.
   However, at the current stage, no further testing around pci-testdev is implemented.
   
   ## Possible issues
   
   (1)
   I ported this from Jailhouse inmate library.
   Most of the APIs were rewritten, but the MSI/MSI-X registration routine and MARCO definitions were copied.
   Although Jailhouse is GPLv2, the inmate library is BSD licensed.
   I think we are good. I did included BSD headers on affected files.
   
   (2)
   I am not a pro on PCI-E, This is originated from my research artifact.
   The API might not be properly definded.
   Suggestions and instructions are welcome.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-624225246


   Did the PR checks fail to start for this one too?  There was a rumor that if you rebase then force push the branch, the checks will start.  Can you try that?
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420716597



##########
File path: drivers/pcie/pcie_root.c
##########
@@ -0,0 +1,358 @@
+/****************************************************************************
+ * nuttx/drivers/pcie/pcie_root.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/pcie/pcie.h>
+#include <nuttx/virt/qemu_pci.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_dev_type_s *pci_device_types[] =
+{
+#ifdef CONFIG_VIRT_QEMU_PCI_TEST
+  &pcie_type_qemu_pci_test,
+#endif /* CONFIG_VIRT_QEMU_PCI_TEST */
+  NULL,
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pcie_initialize
+ *
+ * Description:
+ *  Initialize the PCI-E bus and enumerate the devices with give devices
+ *  type array
+ *
+ * Input Parameters:
+ *   bus    - An PCIE bus
+ *   types  - A array of PCIE device types
+ *   num    - Number of device types
+ *
+ * Returned Value:
+ *   OK if the driver was successfully register; A negated errno value is
+ *   returned on any failure.
+ *
+ ****************************************************************************/
+
+int pcie_initialize(FAR struct pcie_bus_s *bus)
+{
+  return bus->ops->pcie_enumerate(bus, pci_device_types);
+}
+
+/****************************************************************************
+ * Name: pci_enable_device
+ *
+ * Description:
+ *  Enable device with flags
+ *
+ * Input Parameters:
+ *   bdf - device BDF
+ *   flags - device ability to be enabled
+ *
+ * Return value:
+ *   -EINVAL: error
+ *   OK: OK
+ *
+ ****************************************************************************/
+
+int pci_enable_device(FAR struct pcie_dev_s *dev, uint32_t flags)
+{
+  uint16_t old_cmd;
+  uint16_t cmd;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_COMMAND, &old_cmd, 2);
+
+  cmd = old_cmd | flags;
+
+  dev->bus->ops->pci_cfg_write(dev, PCI_CFG_COMMAND, &cmd, 2);
+
+  pciinfo("%02x:%02x.%x, CMD: %x -> %x\n",
+          dev->bdf >> 8, (dev->bdf >> 3) & 0x1f, dev->bdf & 0x3,
+          old_cmd, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_find_cap
+ *
+ * Description:
+ *  Search through the PCI-e device capability list to find given capability.
+ *
+ * Input Parameters:
+ *   dev - Device
+ *   cap - Bitmask of capability
+ *
+ * Returned Value:
+ *   -1: Capability not supported
+ *   other: the offset in PCI configuration space to the capability structure
+ *
+ ****************************************************************************/
+
+int pci_find_cap(FAR struct pcie_dev_s *dev, uint16_t cap)
+{
+  uint8_t pos = PCI_CFG_CAP_PTR - 1;
+  uint16_t status;
+  uint8_t rcap;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_STATUS, &status, 2);
+
+  if (!(status & PCI_STS_CAPS))
+      return -EINVAL;
+
+  while (1)
+    {
+      dev->bus->ops->pci_cfg_read(dev, pos + 1, &pos, 1);
+      if (pos == 0)
+          return -EINVAL;
+
+      dev->bus->ops->pci_cfg_read(dev, pos, &rcap, 1);
+
+      if (rcap == cap)
+          return pos;
+    }
+}
+
+/****************************************************************************
+ * Name: pci_get_bar
+ *
+ * Description:
+ *  Get a 32 bits bar
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   ret    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_get_bar(FAR struct pcie_dev_s *dev, uint32_t bar,
+                uint32_t *ret)
+{
+  if (bar > 5)
+      return -EINVAL;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_BAR + bar * 4, ret, 4);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_get_bar64
+ *
+ * Description:
+ *  Get a 64 bits bar
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   ret    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_get_bar64(FAR struct pcie_dev_s *dev, uint32_t bar,
+                  uint64_t *ret)
+{
+  if (bar > 5 || ((bar % 2) != 0))
+      return -EINVAL;

Review comment:
       yes
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420674125



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
##########
@@ -0,0 +1,229 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The PCI-E Definitions and part of the access routines are taken from
+ * Jailhouse inmate library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+#define __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include <nuttx/board.h>
+#include <nuttx/serial/uart_16550.h>

Review comment:
       By accident, I will remove it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420867067



##########
File path: drivers/pcie/pcie_root.c
##########
@@ -0,0 +1,446 @@
+/****************************************************************************
+ * nuttx/drivers/pcie/pcie_root.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/pcie/pcie.h>
+#include <nuttx/virt/qemu_pci.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_dev_type_s *pci_device_types[] =
+{
+#ifdef CONFIG_VIRT_QEMU_PCI_TEST
+  &pcie_type_qemu_pci_test,
+#endif /* CONFIG_VIRT_QEMU_PCI_TEST */
+  NULL,
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pci_enumerate
+ *
+ * Description:
+ *  Scan the PCI bus and enumerate the devices.
+ *  Initialize any recognized devices, given in types.
+ *
+ * Input Parameters:
+ *   bus    - PCI-E bus structure
+ *   type   - List of pointers to devices types recognized, NULL terminated
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_enumerate(FAR struct pcie_bus_s *bus,
+                  FAR struct pcie_dev_type_s **types)
+{
+  unsigned int bdf;
+  uint16_t vid;
+  uint16_t id;
+  uint16_t rev;
+  struct pcie_dev_s tmp_dev;
+  struct pcie_dev_type_s tmp_type =
+    {
+      .name = "Unknown",
+      .vendor = PCI_ID_ANY,
+      .device = PCI_ID_ANY,
+      .class_rev = PCI_ID_ANY,
+      .probe = NULL,
+    };
+
+  if (!bus)
+      return -EINVAL;
+  if (!types)
+      return -EINVAL;
+
+  for (bdf = 0; bdf < CONFIG_PCIE_MAX_BDF; bdf++)
+    {
+      tmp_dev.bus = bus;
+      tmp_dev.type = &tmp_type;
+      tmp_dev.bdf = bdf;
+
+      bus->ops->pci_cfg_read(&tmp_dev, PCI_CFG_VENDOR_ID, &vid, 2);
+      bus->ops->pci_cfg_read(&tmp_dev, PCI_CFG_DEVICE_ID, &id, 2);
+      bus->ops->pci_cfg_read(&tmp_dev, PCI_CFG_REVERSION, &rev, 2);
+
+      if (vid == PCI_ID_ANY)
+        continue;
+
+      pciinfo("[%02x:%02x.%x] Found %04x:%04x, class/reversion %08x\n",
+              bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3,
+              vid, id, rev);
+
+      for (int i = 0; types[i] != NULL; i++)
+        {
+          if (types[i]->vendor == PCI_ID_ANY ||
+              types[i]->vendor == vid)
+            {
+              if (types[i]->device == PCI_ID_ANY ||
+                  types[i]->device == id)
+                {
+                  if (types[i]->class_rev == PCI_ID_ANY ||
+                      types[i]->class_rev == rev)
+                    {
+                      if (types[i]->probe)
+                        {
+                          pciinfo("[%02x:%02x.%x] %s\n",
+                                  bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3,
+                                  types[i]->name);
+                          types[i]->probe(bus, types[i], bdf);
+                        }
+                      else
+                        {
+                          pcierr("[%02x:%02x.%x] Error: Invalid \
+                                  device probe function\n",
+                                  bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3);
+                        }
+                      break;
+                    }
+                }
+            }
+        }
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pcie_initialize
+ *
+ * Description:
+ *  Initialize the PCI-E bus and enumerate the devices with give devices
+ *  type array
+ *
+ * Input Parameters:
+ *   bus    - An PCIE bus
+ *   types  - A array of PCIE device types
+ *   num    - Number of device types
+ *
+ * Returned Value:
+ *   OK if the driver was successfully register; A negated errno value is
+ *   returned on any failure.
+ *
+ ****************************************************************************/
+
+int pcie_initialize(FAR struct pcie_bus_s *bus)
+{
+  return pci_enumerate(bus, pci_device_types);
+}
+
+/****************************************************************************
+ * Name: pci_enable_device
+ *
+ * Description:
+ *  Enable device with MMIO
+ *
+ * Input Parameters:
+ *   dev - device
+ *
+ * Return value:
+ *   -EINVAL: error
+ *   OK: OK
+ *
+ ****************************************************************************/
+
+int pci_enable_device(FAR struct pcie_dev_s *dev)
+{
+  uint16_t old_cmd;
+  uint16_t cmd;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_COMMAND, &old_cmd, 2);
+
+  cmd = old_cmd | (PCI_CMD_MASTER | PCI_CMD_MEM);
+
+  dev->bus->ops->pci_cfg_write(dev, PCI_CFG_COMMAND, &cmd, 2);
+
+  pciinfo("%02x:%02x.%x, CMD: %x -> %x\n",
+          dev->bdf >> 8, (dev->bdf >> 3) & 0x1f, dev->bdf & 0x3,
+          old_cmd, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_find_cap
+ *
+ * Description:
+ *  Search through the PCI-e device capability list to find given capability.
+ *
+ * Input Parameters:
+ *   dev - Device
+ *   cap - Bitmask of capability
+ *
+ * Returned Value:
+ *   -1: Capability not supported
+ *   other: the offset in PCI configuration space to the capability structure
+ *
+ ****************************************************************************/
+
+int pci_find_cap(FAR struct pcie_dev_s *dev, uint16_t cap)
+{
+  uint8_t pos = PCI_CFG_CAP_PTR - 1;
+  uint16_t status;
+  uint8_t rcap;
+

Review comment:
       Can you be more specific?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420743727



##########
File path: drivers/pcie/pcie_root.c
##########
@@ -0,0 +1,446 @@
+/****************************************************************************
+ * nuttx/drivers/pcie/pcie_root.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/pcie/pcie.h>
+#include <nuttx/virt/qemu_pci.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_dev_type_s *pci_device_types[] =
+{
+#ifdef CONFIG_VIRT_QEMU_PCI_TEST
+  &pcie_type_qemu_pci_test,
+#endif /* CONFIG_VIRT_QEMU_PCI_TEST */
+  NULL,
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pci_enumerate
+ *
+ * Description:
+ *  Scan the PCI bus and enumerate the devices.
+ *  Initialize any recognized devices, given in types.
+ *
+ * Input Parameters:
+ *   bus    - PCI-E bus structure
+ *   type   - List of pointers to devices types recognized, NULL terminated
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_enumerate(FAR struct pcie_bus_s *bus,
+                  FAR struct pcie_dev_type_s **types)
+{
+  unsigned int bdf;
+  uint16_t vid;
+  uint16_t id;
+  uint16_t rev;
+  struct pcie_dev_s tmp_dev;
+  struct pcie_dev_type_s tmp_type =
+    {
+      .name = "Unknown",
+      .vendor = PCI_ID_ANY,
+      .device = PCI_ID_ANY,
+      .class_rev = PCI_ID_ANY,
+      .probe = NULL,
+    };
+
+  if (!bus)
+      return -EINVAL;
+  if (!types)
+      return -EINVAL;
+
+  for (bdf = 0; bdf < CONFIG_PCIE_MAX_BDF; bdf++)
+    {
+      tmp_dev.bus = bus;
+      tmp_dev.type = &tmp_type;
+      tmp_dev.bdf = bdf;
+
+      bus->ops->pci_cfg_read(&tmp_dev, PCI_CFG_VENDOR_ID, &vid, 2);
+      bus->ops->pci_cfg_read(&tmp_dev, PCI_CFG_DEVICE_ID, &id, 2);
+      bus->ops->pci_cfg_read(&tmp_dev, PCI_CFG_REVERSION, &rev, 2);
+
+      if (vid == PCI_ID_ANY)
+        continue;
+
+      pciinfo("[%02x:%02x.%x] Found %04x:%04x, class/reversion %08x\n",
+              bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3,
+              vid, id, rev);
+
+      for (int i = 0; types[i] != NULL; i++)
+        {
+          if (types[i]->vendor == PCI_ID_ANY ||
+              types[i]->vendor == vid)
+            {
+              if (types[i]->device == PCI_ID_ANY ||
+                  types[i]->device == id)
+                {
+                  if (types[i]->class_rev == PCI_ID_ANY ||
+                      types[i]->class_rev == rev)
+                    {
+                      if (types[i]->probe)
+                        {
+                          pciinfo("[%02x:%02x.%x] %s\n",
+                                  bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3,
+                                  types[i]->name);
+                          types[i]->probe(bus, types[i], bdf);
+                        }
+                      else
+                        {
+                          pcierr("[%02x:%02x.%x] Error: Invalid \
+                                  device probe function\n",
+                                  bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3);
+                        }
+                      break;
+                    }
+                }
+            }
+        }
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pcie_initialize
+ *
+ * Description:
+ *  Initialize the PCI-E bus and enumerate the devices with give devices
+ *  type array
+ *
+ * Input Parameters:
+ *   bus    - An PCIE bus
+ *   types  - A array of PCIE device types
+ *   num    - Number of device types
+ *
+ * Returned Value:
+ *   OK if the driver was successfully register; A negated errno value is
+ *   returned on any failure.
+ *
+ ****************************************************************************/
+
+int pcie_initialize(FAR struct pcie_bus_s *bus)
+{
+  return pci_enumerate(bus, pci_device_types);
+}
+
+/****************************************************************************
+ * Name: pci_enable_device
+ *
+ * Description:
+ *  Enable device with MMIO
+ *
+ * Input Parameters:
+ *   dev - device
+ *
+ * Return value:
+ *   -EINVAL: error
+ *   OK: OK
+ *
+ ****************************************************************************/
+
+int pci_enable_device(FAR struct pcie_dev_s *dev)
+{
+  uint16_t old_cmd;
+  uint16_t cmd;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_COMMAND, &old_cmd, 2);
+
+  cmd = old_cmd | (PCI_CMD_MASTER | PCI_CMD_MEM);
+
+  dev->bus->ops->pci_cfg_write(dev, PCI_CFG_COMMAND, &cmd, 2);
+
+  pciinfo("%02x:%02x.%x, CMD: %x -> %x\n",
+          dev->bdf >> 8, (dev->bdf >> 3) & 0x1f, dev->bdf & 0x3,
+          old_cmd, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_find_cap
+ *
+ * Description:
+ *  Search through the PCI-e device capability list to find given capability.
+ *
+ * Input Parameters:
+ *   dev - Device
+ *   cap - Bitmask of capability
+ *
+ * Returned Value:
+ *   -1: Capability not supported
+ *   other: the offset in PCI configuration space to the capability structure
+ *
+ ****************************************************************************/
+
+int pci_find_cap(FAR struct pcie_dev_s *dev, uint16_t cap)
+{
+  uint8_t pos = PCI_CFG_CAP_PTR - 1;
+  uint16_t status;
+  uint8_t rcap;
+

Review comment:
       we must judge the pci_cfg_read whether is NULL.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420678056



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
##########
@@ -0,0 +1,229 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The PCI-E Definitions and part of the access routines are taken from
+ * Jailhouse inmate library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+#define __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include <nuttx/board.h>
+#include <nuttx/serial/uart_16550.h>
+#include <arch/board/board.h>
+
+#include "up_arch.h"
+#include "up_internal.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define PCI_REG_ADDR_PORT       0xcf8
+#define PCI_REG_DATA_PORT       0xcfc
+
+#define PCI_CONE                (1 << 31)
+
+/****************************************************************************
+ * Name: __qemu_pci_cfg_write
+ *
+ * Description:
+ *  Write 8, 16, 32 bits data to PCI-E configuration space of device
+ *  specified by dev
+ *
+ * Input Parameters:
+ *   bfd    - Device private data
+ *   buffer - A pointer to the read-only buffer of data to be written
+ *   size   - The number of bytes to send from the buffer
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static inline int __qemu_pci_cfg_write(uint16_t bfd, uintptr_t addr,
+                                       FAR const void *buffer,
+                                       unsigned int size)
+{

Review comment:
       Yes, I mean that, do we need to judge the buffer whether is NULL?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-624422211


   Oh, good. It's running now.
   I did some cosmetic changes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625210301


   I am quite uncertain that `pci_cfg_read` should directly return the value or not. 
   Need suggestions.
   
   Yes, it more convenient to use and the semantics are much better if we driectly return the content.
   However, I get quite paranoid about the fact that someone might give an invalid size to read.
   
   Of course, we can make 4 functions: read8, read16, read32 and read64, but the code will bloat.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420663754



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
##########
@@ -0,0 +1,461 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The MSI and MSI-X vector setup function are taken from Jailhouse inmate
+ * library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include "qemu_pcie_readwrite.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define QEMU_PCIE_MAX_BDF 0x10000
+
+/****************************************************************************
+ * Private Functions Definitions
+ ****************************************************************************/
+
+static int qemu_pci_enumerate(FAR struct pcie_bus_s *bus,
+                               FAR struct pcie_dev_type_s **types);
+
+static int qemu_pci_cfg_write(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                              FAR const void *buffer, unsigned int size);
+
+static int qemu_pci_cfg_read(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                             FAR void *buffer, unsigned int size);
+
+static int qemu_pci_map_bar(FAR struct pcie_dev_s *dev, uint32_t addr,
+                            unsigned long length);
+
+static int qemu_pci_map_bar64(FAR struct pcie_dev_s *dev, uint64_t addr,
+                              unsigned long length);
+
+static int qemu_pci_msix_register(FAR struct pcie_dev_s *dev,
+                                  uint32_t vector, uint32_t index);
+
+static int qemu_pci_msi_register(FAR struct pcie_dev_s *dev,
+                                 uint16_t vector);
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_bus_ops_s qemu_pcie_bus_ops =
+{
+    .pcie_enumerate    =   qemu_pci_enumerate,
+    .pci_cfg_write     =   qemu_pci_cfg_write,
+    .pci_cfg_read      =   qemu_pci_cfg_read,
+    .pci_map_bar       =   qemu_pci_map_bar,
+    .pci_map_bar64     =   qemu_pci_map_bar64,
+    .pci_msix_register = qemu_pci_msix_register,
+    .pci_msi_register  = qemu_pci_msi_register,
+};
+
+struct pcie_bus_s qemu_pcie_bus =
+{
+    .ops = &qemu_pcie_bus_ops,
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: qemu_pci_enumerate
+ *
+ * Description:
+ *  Scan the PCI bus and enumerate the devices.
+ *  Initialize any recognized devices, given in types.
+ *
+ * Input Parameters:
+ *   bus    - PCI-E bus structure
+ *   type   - List of pointers to devices types recognized, NULL terminated
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+

Review comment:
       the enumerate function is common code, I think move this function to drivers/pcie is better, so we need to create a folder in drivers.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] btashton edited a comment on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
btashton edited a comment on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625323858


   > I am quite uncertain that `pci_cfg_read` should directly return the value or not. 
   > Need suggestions.
   > 
   > Yes, it more convenient to use and the semantics are much better if we driectly return the content.
   > However, I get quite paranoid about the fact that someone might give an invalid size to read.
   > 
   > Of course, we can make 4 functions: read8, read16, read32 and read64, but the code will bloat.
   > 
   > 
   For what it is worth freebsd uses basically the same prototype. 
   
   ```
   uint32_t
        pcie_read_config(device_t dev, int	reg, int width);
   ```
   
   I think if we put a debug assert in we should be fine since the code is broken either way.  I don't see a case where the read or write length should be dynamic. 
   
   While I'm not following this exactly I am using this for inspiration of interfaces.  https://www.freebsd.org/cgi/man.cgi?query=pci&sektion=9


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420867515



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
##########
@@ -0,0 +1,467 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The MSI and MSI-X vector setup function are taken from Jailhouse inmate
+ * library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include "qemu_pcie_readwrite.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define QEMU_PCIE_MAX_BDF 0x10000
+
+/****************************************************************************
+ * Private Functions Definitions
+ ****************************************************************************/
+
+static int qemu_pci_enumerate(FAR struct pcie_bus_s *bus,
+                               FAR struct pcie_dev_type_s **types);
+
+static int qemu_pci_cfg_write(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                              FAR const void *buffer, unsigned int size);
+
+static int qemu_pci_cfg_read(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                             FAR void *buffer, unsigned int size);
+
+static int qemu_pci_map_bar(FAR struct pcie_dev_s *dev, uint32_t addr,
+                            unsigned long length);
+
+static int qemu_pci_map_bar64(FAR struct pcie_dev_s *dev, uint64_t addr,
+                              unsigned long length);
+
+static int qemu_pci_msix_register(FAR struct pcie_dev_s *dev,
+                                  uint32_t vector, uint32_t index);
+
+static int qemu_pci_msi_register(FAR struct pcie_dev_s *dev,
+                                 uint16_t vector);
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_bus_ops_s qemu_pcie_bus_ops =
+{
+    .pcie_enumerate    =   qemu_pci_enumerate,
+    .pci_cfg_write     =   qemu_pci_cfg_write,
+    .pci_cfg_read      =   qemu_pci_cfg_read,
+    .pci_map_bar       =   qemu_pci_map_bar,
+    .pci_map_bar64     =   qemu_pci_map_bar64,
+    .pci_msix_register = qemu_pci_msix_register,
+    .pci_msi_register  = qemu_pci_msi_register,
+};
+
+struct pcie_bus_s qemu_pcie_bus =
+{
+    .ops = &qemu_pcie_bus_ops,
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: qemu_pci_enumerate
+ *
+ * Description:
+ *  Scan the PCI bus and enumerate the devices.
+ *  Initialize any recognized devices, given in types.
+ *
+ * Input Parameters:
+ *   bus    - PCI-E bus structure
+ *   type   - List of pointers to devices types recognized, NULL terminated
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static int qemu_pci_enumerate(FAR struct pcie_bus_s *bus,
+                               FAR struct pcie_dev_type_s **types)
+{
+  unsigned int bdf;
+  uint16_t vid;
+  uint16_t id;
+  uint16_t rev;
+
+  if (!bus)
+      return -EINVAL;
+  if (!types)
+      return -EINVAL;
+
+  for (bdf = 0; bdf < QEMU_PCIE_MAX_BDF; bdf++)
+    {
+      __qemu_pci_cfg_read(bdf, PCI_CFG_VENDOR_ID, &vid, 2);
+      __qemu_pci_cfg_read(bdf, PCI_CFG_DEVICE_ID, &id, 2);
+      __qemu_pci_cfg_read(bdf, PCI_CFG_REVERSION, &rev, 2);
+
+      if (vid == PCI_ID_ANY)
+        continue;
+
+      pciinfo("[%02x:%02x.%x] Found %04x:%04x, class/reversion %08x\n",
+              bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3,
+              vid, id, rev);
+
+      for (int i = 0; types[i] != NULL; i++)
+        {
+          if (types[i]->vendor == PCI_ID_ANY ||
+              types[i]->vendor == vid)
+            {
+              if (types[i]->device == PCI_ID_ANY ||
+                  types[i]->device == id)
+                {
+                  if (types[i]->class_rev == PCI_ID_ANY ||
+                      types[i]->class_rev == rev)
+                    {
+                      if (types[i]->probe)
+                        {
+                          pciinfo("[%02x:%02x.%x] %s\n",
+                                  bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3,
+                                  types[i]->name);
+                          types[i]->probe(bus, types[i], bdf);
+                        }
+                      else
+                        {
+                          pcierr("[%02x:%02x.%x] Error: Invalid \
+                                  device probe function\n",
+                                  bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3);
+                        }
+                      break;
+                    }
+                }
+            }
+        }
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: qemu_pci_cfg_write
+ *
+ * Description:
+ *  Write 8, 16, 32, 64 bits data to PCI-E configuration space of device
+ *  specified by dev
+ *
+ * Input Parameters:
+ *   bdf    - Device private data
+ *   buffer - A pointer to the read-only buffer of data to be written
+ *   size   - The number of bytes to send from the buffer
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static int qemu_pci_cfg_write(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                              FAR const void *buffer, unsigned int size)
+{
+  if(!buffer)
+      return -EINVAL;
+
+  switch (size)
+    {
+      case 1:
+      case 2:
+      case 4:
+        return __qemu_pci_cfg_write(dev->bdf, addr, buffer, size);
+      case 8:
+        return __qemu_pci_cfg_write(dev->bdf, addr, buffer, size);
+      default:
+        return -EINVAL;
+    }
+}
+
+/****************************************************************************
+ * Name: qemu_pci_cfg_read
+ *
+ * Description:
+ *  Read 8, 16, 32, 64 bits data from PCI-E configuration space of device
+ *  specified by dev
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   buffer - A pointer to a buffer to receive the data from the device
+ *   size   - The requested number of bytes to be read
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static int qemu_pci_cfg_read(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                             FAR void *buffer, unsigned int size)
+{
+  if(!buffer)
+      return -EINVAL;
+
+  switch (size)
+    {
+      case 1:
+      case 2:
+      case 4:
+        return __qemu_pci_cfg_read(dev->bdf, addr, buffer, size);
+      case 8:
+        return __qemu_pci_cfg_read64(dev->bdf, addr, buffer, size);
+      default:
+        return -EINVAL;
+    }
+}
+
+/****************************************************************************
+ * Name: qemu_pci_map_bar
+ *
+ * Description:
+ *  Map address in a 32 bits bar in the memory address space
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   length - Map length, multiple of PAGE_SIZE
+ *   ret    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static int qemu_pci_map_bar(FAR struct pcie_dev_s *dev, uint32_t addr,
+                            unsigned long length)
+{
+  up_map_region((void *)((uintptr_t)addr), length,
+      X86_PAGE_WR | X86_PAGE_PRESENT | X86_PAGE_NOCACHE | X86_PAGE_GLOBAL);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: qemu_pci_map_bar64
+ *
+ * Description:
+ *  Map address in a 64 bits bar in the memory address space
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   length - Map length, multiple of PAGE_SIZE
+ *   ret    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static int qemu_pci_map_bar64(FAR struct pcie_dev_s *dev, uint64_t addr,
+                              unsigned long length)
+{
+  up_map_region((void *)((uintptr_t)addr), length,
+      X86_PAGE_WR | X86_PAGE_PRESENT | X86_PAGE_NOCACHE | X86_PAGE_GLOBAL);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: qemu_pci_msix_register
+ *
+ * Description:
+ *  Map a device MSI-X vector to a platform IRQ vector
+ *
+ * Input Parameters:
+ *   dev - Device
+ *   vector - IRQ number of the platform
+ *   index  - Device MSI-X vector number
+ *
+ * Returned Value:
+ *   <0: Mapping failed
+ *    0: Mapping succeed
+ *
+ ****************************************************************************/
+
+static int qemu_pci_msix_register(FAR struct pcie_dev_s *dev,
+                                  uint32_t vector, uint32_t index)
+{
+  unsigned int bar;
+  uint16_t message_control;
+  uint32_t table_bar_ind;
+  uint32_t lo_table_addr;
+  uint32_t hi_table_addr;
+  uint64_t msix_table_addr = 0;
+
+  int cap = pci_find_cap(dev, PCI_CAP_MSIX);
+  if (cap < 0)
+      return -EINVAL;
+

Review comment:
       I created a patch replaced the MSI/MSIX related offset values and bit fields with marcos.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] btashton commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625323858


   > I am quite uncertain that `pci_cfg_read` should directly return the value or not. 
   > Need suggestions.
   > 
   > Yes, it more convenient to use and the semantics are much better if we driectly return the content.
   > However, I get quite paranoid about the fact that someone might give an invalid size to read.
   > 
   > Of course, we can make 4 functions: read8, read16, read32 and read64, but the code will bloat.
   > 
   > 
   For what it is worth freebsd uses basically the same prototype. 
   
   ```
   uint32_t
        pcie_read_config(device_t dev, int	reg, int width);
   ```
   
   I think if we put a debug assert in we should be fine since the code is broken either way.  I don't see a case where the read or write length should be dynamic. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r421437126



##########
File path: drivers/pcie/pcie_root.c
##########
@@ -0,0 +1,446 @@
+/****************************************************************************
+ * nuttx/drivers/pcie/pcie_root.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/pcie/pcie.h>
+#include <nuttx/virt/qemu_pci.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_dev_type_s *pci_device_types[] =
+{
+#ifdef CONFIG_VIRT_QEMU_PCI_TEST
+  &pcie_type_qemu_pci_test,
+#endif /* CONFIG_VIRT_QEMU_PCI_TEST */
+  NULL,
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pci_enumerate
+ *
+ * Description:
+ *  Scan the PCI bus and enumerate the devices.
+ *  Initialize any recognized devices, given in types.
+ *
+ * Input Parameters:
+ *   bus    - PCI-E bus structure
+ *   type   - List of pointers to devices types recognized, NULL terminated
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_enumerate(FAR struct pcie_bus_s *bus,
+                  FAR struct pcie_dev_type_s **types)
+{
+  unsigned int bdf;
+  uint16_t vid;
+  uint16_t id;
+  uint16_t rev;
+  struct pcie_dev_s tmp_dev;
+  struct pcie_dev_type_s tmp_type =
+    {
+      .name = "Unknown",
+      .vendor = PCI_ID_ANY,
+      .device = PCI_ID_ANY,
+      .class_rev = PCI_ID_ANY,
+      .probe = NULL,
+    };
+
+  if (!bus)
+      return -EINVAL;
+  if (!types)
+      return -EINVAL;
+
+  for (bdf = 0; bdf < CONFIG_PCIE_MAX_BDF; bdf++)
+    {
+      tmp_dev.bus = bus;
+      tmp_dev.type = &tmp_type;
+      tmp_dev.bdf = bdf;
+
+      bus->ops->pci_cfg_read(&tmp_dev, PCI_CFG_VENDOR_ID, &vid, 2);
+      bus->ops->pci_cfg_read(&tmp_dev, PCI_CFG_DEVICE_ID, &id, 2);
+      bus->ops->pci_cfg_read(&tmp_dev, PCI_CFG_REVERSION, &rev, 2);
+
+      if (vid == PCI_ID_ANY)
+        continue;
+
+      pciinfo("[%02x:%02x.%x] Found %04x:%04x, class/reversion %08x\n",
+              bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3,
+              vid, id, rev);
+
+      for (int i = 0; types[i] != NULL; i++)
+        {
+          if (types[i]->vendor == PCI_ID_ANY ||
+              types[i]->vendor == vid)
+            {
+              if (types[i]->device == PCI_ID_ANY ||
+                  types[i]->device == id)
+                {
+                  if (types[i]->class_rev == PCI_ID_ANY ||
+                      types[i]->class_rev == rev)
+                    {
+                      if (types[i]->probe)
+                        {
+                          pciinfo("[%02x:%02x.%x] %s\n",
+                                  bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3,
+                                  types[i]->name);
+                          types[i]->probe(bus, types[i], bdf);
+                        }
+                      else
+                        {
+                          pcierr("[%02x:%02x.%x] Error: Invalid \
+                                  device probe function\n",
+                                  bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3);
+                        }
+                      break;
+                    }
+                }
+            }
+        }
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pcie_initialize
+ *
+ * Description:
+ *  Initialize the PCI-E bus and enumerate the devices with give devices
+ *  type array
+ *
+ * Input Parameters:
+ *   bus    - An PCIE bus
+ *   types  - A array of PCIE device types
+ *   num    - Number of device types
+ *
+ * Returned Value:
+ *   OK if the driver was successfully register; A negated errno value is
+ *   returned on any failure.
+ *
+ ****************************************************************************/
+
+int pcie_initialize(FAR struct pcie_bus_s *bus)
+{
+  return pci_enumerate(bus, pci_device_types);
+}
+
+/****************************************************************************
+ * Name: pci_enable_device
+ *
+ * Description:
+ *  Enable device with MMIO
+ *
+ * Input Parameters:
+ *   dev - device
+ *
+ * Return value:
+ *   -EINVAL: error
+ *   OK: OK
+ *
+ ****************************************************************************/
+
+int pci_enable_device(FAR struct pcie_dev_s *dev)
+{
+  uint16_t old_cmd;
+  uint16_t cmd;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_COMMAND, &old_cmd, 2);
+
+  cmd = old_cmd | (PCI_CMD_MASTER | PCI_CMD_MEM);
+
+  dev->bus->ops->pci_cfg_write(dev, PCI_CFG_COMMAND, &cmd, 2);
+
+  pciinfo("%02x:%02x.%x, CMD: %x -> %x\n",
+          dev->bdf >> 8, (dev->bdf >> 3) & 0x1f, dev->bdf & 0x3,
+          old_cmd, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_find_cap
+ *
+ * Description:
+ *  Search through the PCI-e device capability list to find given capability.
+ *
+ * Input Parameters:
+ *   dev - Device
+ *   cap - Bitmask of capability
+ *
+ * Returned Value:
+ *   -1: Capability not supported
+ *   other: the offset in PCI configuration space to the capability structure
+ *
+ ****************************************************************************/
+
+int pci_find_cap(FAR struct pcie_dev_s *dev, uint16_t cap)
+{
+  uint8_t pos = PCI_CFG_CAP_PTR - 1;
+  uint16_t status;
+  uint8_t rcap;
+

Review comment:
       I see.
   
   Maybe and assert is more proper?
   
   It's wired that this is not implemented when using PCI-E.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] btashton edited a comment on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
btashton edited a comment on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625159985


   @sonicyang I did some work here https://github.com/btashton/incubator-nuttx/tree/pci-btashton
   I have ported over a good chunk of the code that I wrote before in place of the Jailhouse code.  I have not pulled in any of my msi/msix code.  I did change the logic around enumeration to support bridges as well as be smarter about traversing the space.
   
   I also moved a lot of the logic that was not pci-e specific to just pci.
   
   https://github.com/btashton/incubator-nuttx/tree/pci-btashton
   Let me know what you think and I can keep porting over more of the logic if it makes sense.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang edited a comment on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang edited a comment on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625793332


   @btashton I have checked your modifications.
   I didn't make it work with my serial card because of lacking find_cap and msi/msix routines.
   
   This is to be expected. I am reworking those right now, but should have that done this weekend.
   
   But I did confirmed that it did scan the bus without crashing my Intel Xeon machine.
   
   Great!  I have only been messing with this via QEMU partially because it lets be create custom bus hierarchies.
   
   I did noticed this "smart" way of scanning bus didn't work with Jailhouse, because Jailhouse didn't have a full PCI hierarchy, e.g. root device, etc. Devices are passed to the cells via VT-d separately.
   
   This should not be a problem, but I did see that Jailhouse breaks the PCI spec in a way I did not check for.  They let you pass individual device functions which means that when I check func 0 I will stop looking. I bet this is where things went wrong.  I added a patch to work around this to my branch.  I it would be great if you could see if it finds your devices.
   
   I am currently working on ivshmem device driver for Jailhouse, I hope we can take this problem into account.
   
   We certainly should handle this case.  I made a note in my patch about how to detect Jailhouse to work around this issue (for now I hard coded it on)
   
   I purpose that we should open a PCI feature branch, cooperate there with PR.
   Of course, we need some one with permission to do this.
   
   I created the branch off of what was in this PR called pci. https://github.com/apache/incubator-nuttx/tree/pci
   I think this is a great starting point and we can review it more as a whole once we have it more mature.  I also don't want to block you while I am working on some more of the low level primitives without the jailroot code. I also have a PCIe FPGA board that should be here in a few weeks that will allow me to test some of this against a RISC-V softcore.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625793332


   @btashton I have checked your modifications.
   I didn't make it work with my serial card because of lacking find_cap and msi/msix routines.
   But I did confirmed that it did scan the bus without crashing my Intel Xeon machine.
   
   I did noticed this "smart" way of scanning bus didn't work with Jailhouse, because Jailhouse didn't have a full PCI hierarchy, e.g. root device, etc. Devices are passed to the cells via VT-d separately.
   
   I am currently working on ivshmem device driver for Jailhouse, I hope we can take this problem into account.
   
   I purpose that we should open a PCI feature branch, cooperate there with PR.
   Of course, we need some one with permission to do this.
   
   What do you think?
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-624420616


   The affected list
    * pre-processor definitions
    * pci_find_cap()
    * qemu_pci_enumerate()
    * qemu_pci_msix_register()
    * qemu_pci_msi_register()
    * Whole qemu_pcie_readwrite.h file
    * driver/virt/qemu_pci_test.c has a structure definition taken from qemu documentation.
   
   I can do a full rewrite, but I doubt those has a totally different implementation though.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] btashton edited a comment on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
btashton edited a comment on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625844424


   @sonicyang -- First off sorry I clicked edit on your comment instead of reply, should have restored it back to what you had.
   
   > @btashton I have checked your modifications.
   > I didn't make it work with my serial card because of lacking find_cap and msi/msix routines.
   
   This is to be expected. I am reworking those right now, but should have that done this weekend.
   
   > But I did confirmed that it did scan the bus without crashing my Intel Xeon machine.
   
   Great! I have only been messing with this via QEMU partially because it lets be create custom bus hierarchies.
   
   > 
   > I did noticed this "smart" way of scanning bus didn't work with Jailhouse, because Jailhouse didn't have a full PCI hierarchy, e.g. root device, etc. Devices are passed to the cells via VT-d separately.
   
   This should not be a problem, but I did see that Jailhouse breaks the PCI spec in a way I did not check for. They let you pass individual device functions which means that when I check func 0 I will stop looking. I bet this is where things went wrong. I added a patch to work around this to my branch. I it would be great if you could see if it finds your devices.
   
   > 
   > I am currently working on ivshmem device driver for Jailhouse, I hope we can take this problem into account.
   
   We certainly should handle this case. I made a note in my patch about how to detect Jailhouse to work around this issue (for now I hard coded it on)
   
   > 
   > I purpose that we should open a PCI feature branch, cooperate there with PR.
   > Of course, we need some one with permission to do this.
   > 
   
   I created the branch off of what was in this PR called pci. https://github.com/apache/incubator-nuttx/tree/pci
   I think this is a great starting point and we can review it more as a whole once we have it more mature. I also don't want to block you while I am working on some more of the low level primitives without the Jailhouse code. I also have a PCIe FPGA board that should be here in a few weeks that will allow me to test some of this against a RISC-V softcore.
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420648077



##########
File path: drivers/pcie/pcie_root.c
##########
@@ -0,0 +1,358 @@
+/****************************************************************************
+ * nuttx/drivers/pcie/pcie_root.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/pcie/pcie.h>
+#include <nuttx/virt/qemu_pci.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_dev_type_s *pci_device_types[] =
+{
+#ifdef CONFIG_VIRT_QEMU_PCI_TEST
+  &pcie_type_qemu_pci_test,
+#endif /* CONFIG_VIRT_QEMU_PCI_TEST */
+  NULL,
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pcie_initialize
+ *
+ * Description:
+ *  Initialize the PCI-E bus and enumerate the devices with give devices
+ *  type array
+ *
+ * Input Parameters:
+ *   bus    - An PCIE bus
+ *   types  - A array of PCIE device types
+ *   num    - Number of device types
+ *
+ * Returned Value:
+ *   OK if the driver was successfully register; A negated errno value is
+ *   returned on any failure.
+ *
+ ****************************************************************************/
+
+int pcie_initialize(FAR struct pcie_bus_s *bus)
+{
+  return bus->ops->pcie_enumerate(bus, pci_device_types);
+}
+
+/****************************************************************************
+ * Name: pci_enable_device
+ *
+ * Description:
+ *  Enable device with flags
+ *
+ * Input Parameters:
+ *   bdf - device BDF
+ *   flags - device ability to be enabled
+ *
+ * Return value:
+ *   -EINVAL: error
+ *   OK: OK
+ *
+ ****************************************************************************/
+
+int pci_enable_device(FAR struct pcie_dev_s *dev, uint32_t flags)
+{
+  uint16_t old_cmd;
+  uint16_t cmd;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_COMMAND, &old_cmd, 2);
+
+  cmd = old_cmd | flags;
+
+  dev->bus->ops->pci_cfg_write(dev, PCI_CFG_COMMAND, &cmd, 2);
+
+  pciinfo("%02x:%02x.%x, CMD: %x -> %x\n",
+          dev->bdf >> 8, (dev->bdf >> 3) & 0x1f, dev->bdf & 0x3,
+          old_cmd, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_find_cap
+ *
+ * Description:
+ *  Search through the PCI-e device capability list to find given capability.
+ *
+ * Input Parameters:
+ *   dev - Device
+ *   cap - Bitmask of capability
+ *
+ * Returned Value:
+ *   -1: Capability not supported
+ *   other: the offset in PCI configuration space to the capability structure
+ *
+ ****************************************************************************/
+
+int pci_find_cap(FAR struct pcie_dev_s *dev, uint16_t cap)
+{
+  uint8_t pos = PCI_CFG_CAP_PTR - 1;
+  uint16_t status;
+  uint8_t rcap;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_STATUS, &status, 2);
+
+  if (!(status & PCI_STS_CAPS))
+      return -EINVAL;
+
+  while (1)
+    {
+      dev->bus->ops->pci_cfg_read(dev, pos + 1, &pos, 1);
+      if (pos == 0)
+          return -EINVAL;
+
+      dev->bus->ops->pci_cfg_read(dev, pos, &rcap, 1);
+
+      if (rcap == cap)
+          return pos;
+    }
+}
+
+/****************************************************************************
+ * Name: pci_get_bar
+ *
+ * Description:
+ *  Get a 32 bits bar
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   ret    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_get_bar(FAR struct pcie_dev_s *dev, uint32_t bar,
+                uint32_t *ret)
+{
+  if (bar > 5)
+      return -EINVAL;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_BAR + bar * 4, ret, 4);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_get_bar64
+ *
+ * Description:
+ *  Get a 64 bits bar
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   ret    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_get_bar64(FAR struct pcie_dev_s *dev, uint32_t bar,
+                  uint64_t *ret)
+{
+  if (bar > 5 || ((bar % 2) != 0))
+      return -EINVAL;

Review comment:
       You mean the following is sufficient?
   ```
     if (bar > 4 || ((bar % 2) != 0))
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625201838


   @btashton Sounds fantastic! I will take a look, but I am kinda of exhausted today.
   I will reply on weekend
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420670706



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
##########
@@ -0,0 +1,229 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The PCI-E Definitions and part of the access routines are taken from
+ * Jailhouse inmate library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+#define __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include <nuttx/board.h>
+#include <nuttx/serial/uart_16550.h>

Review comment:
       why add the uart_16550.h, for printing?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420707924



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
##########
@@ -0,0 +1,461 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The MSI and MSI-X vector setup function are taken from Jailhouse inmate
+ * library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include "qemu_pcie_readwrite.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define QEMU_PCIE_MAX_BDF 0x10000
+
+/****************************************************************************
+ * Private Functions Definitions
+ ****************************************************************************/
+
+static int qemu_pci_enumerate(FAR struct pcie_bus_s *bus,
+                               FAR struct pcie_dev_type_s **types);
+
+static int qemu_pci_cfg_write(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                              FAR const void *buffer, unsigned int size);
+
+static int qemu_pci_cfg_read(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                             FAR void *buffer, unsigned int size);
+
+static int qemu_pci_map_bar(FAR struct pcie_dev_s *dev, uint32_t addr,
+                            unsigned long length);
+
+static int qemu_pci_map_bar64(FAR struct pcie_dev_s *dev, uint64_t addr,
+                              unsigned long length);
+
+static int qemu_pci_msix_register(FAR struct pcie_dev_s *dev,
+                                  uint32_t vector, uint32_t index);
+
+static int qemu_pci_msi_register(FAR struct pcie_dev_s *dev,
+                                 uint16_t vector);
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_bus_ops_s qemu_pcie_bus_ops =
+{
+    .pcie_enumerate    =   qemu_pci_enumerate,
+    .pci_cfg_write     =   qemu_pci_cfg_write,
+    .pci_cfg_read      =   qemu_pci_cfg_read,
+    .pci_map_bar       =   qemu_pci_map_bar,
+    .pci_map_bar64     =   qemu_pci_map_bar64,
+    .pci_msix_register = qemu_pci_msix_register,
+    .pci_msi_register  = qemu_pci_msi_register,
+};
+
+struct pcie_bus_s qemu_pcie_bus =
+{
+    .ops = &qemu_pcie_bus_ops,
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: qemu_pci_enumerate
+ *
+ * Description:
+ *  Scan the PCI bus and enumerate the devices.
+ *  Initialize any recognized devices, given in types.
+ *
+ * Input Parameters:
+ *   bus    - PCI-E bus structure
+ *   type   - List of pointers to devices types recognized, NULL terminated
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+

Review comment:
       I get the point, I will try to get it refactored as common code.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420673764



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
##########
@@ -0,0 +1,461 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The MSI and MSI-X vector setup function are taken from Jailhouse inmate
+ * library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include "qemu_pcie_readwrite.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define QEMU_PCIE_MAX_BDF 0x10000
+
+/****************************************************************************
+ * Private Functions Definitions
+ ****************************************************************************/
+
+static int qemu_pci_enumerate(FAR struct pcie_bus_s *bus,
+                               FAR struct pcie_dev_type_s **types);
+
+static int qemu_pci_cfg_write(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                              FAR const void *buffer, unsigned int size);
+
+static int qemu_pci_cfg_read(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                             FAR void *buffer, unsigned int size);
+
+static int qemu_pci_map_bar(FAR struct pcie_dev_s *dev, uint32_t addr,
+                            unsigned long length);
+
+static int qemu_pci_map_bar64(FAR struct pcie_dev_s *dev, uint64_t addr,
+                              unsigned long length);
+
+static int qemu_pci_msix_register(FAR struct pcie_dev_s *dev,
+                                  uint32_t vector, uint32_t index);
+
+static int qemu_pci_msi_register(FAR struct pcie_dev_s *dev,
+                                 uint16_t vector);
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_bus_ops_s qemu_pcie_bus_ops =
+{
+    .pcie_enumerate    =   qemu_pci_enumerate,
+    .pci_cfg_write     =   qemu_pci_cfg_write,
+    .pci_cfg_read      =   qemu_pci_cfg_read,
+    .pci_map_bar       =   qemu_pci_map_bar,
+    .pci_map_bar64     =   qemu_pci_map_bar64,
+    .pci_msix_register = qemu_pci_msix_register,
+    .pci_msi_register  = qemu_pci_msi_register,
+};
+
+struct pcie_bus_s qemu_pcie_bus =
+{
+    .ops = &qemu_pcie_bus_ops,
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: qemu_pci_enumerate
+ *
+ * Description:
+ *  Scan the PCI bus and enumerate the devices.
+ *  Initialize any recognized devices, given in types.
+ *
+ * Input Parameters:
+ *   bus    - PCI-E bus structure
+ *   type   - List of pointers to devices types recognized, NULL terminated
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+

Review comment:
       However, on x86 we can ask BIOS/EFI about the devices.
   On ARM and other, I am not sure.
   
   If they are common, then we can move it to drivers/pcie.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] patacongo commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-624225246


   Did the PR checks fail to start for this one too?
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] btashton commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-624247753


   > Did the PR checks fail to start for this one too? There was a rumor that if you rebase then force push the branch, the checks will start. Can you try that?
   Seems like if there is any issues on github's side you get stuck having to re push to trigger https://github.community/t5/How-to-use-Git-and-GitHub/Is-it-possible-to-manually-force-an-action-workflow-to-be-re-run/m-p/54996/highlight/true#M11604


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420679740



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
##########
@@ -0,0 +1,461 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The MSI and MSI-X vector setup function are taken from Jailhouse inmate
+ * library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include "qemu_pcie_readwrite.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define QEMU_PCIE_MAX_BDF 0x10000
+
+/****************************************************************************
+ * Private Functions Definitions
+ ****************************************************************************/
+
+static int qemu_pci_enumerate(FAR struct pcie_bus_s *bus,
+                               FAR struct pcie_dev_type_s **types);
+
+static int qemu_pci_cfg_write(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                              FAR const void *buffer, unsigned int size);
+
+static int qemu_pci_cfg_read(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                             FAR void *buffer, unsigned int size);
+
+static int qemu_pci_map_bar(FAR struct pcie_dev_s *dev, uint32_t addr,
+                            unsigned long length);
+
+static int qemu_pci_map_bar64(FAR struct pcie_dev_s *dev, uint64_t addr,
+                              unsigned long length);
+
+static int qemu_pci_msix_register(FAR struct pcie_dev_s *dev,
+                                  uint32_t vector, uint32_t index);
+
+static int qemu_pci_msi_register(FAR struct pcie_dev_s *dev,
+                                 uint16_t vector);
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_bus_ops_s qemu_pcie_bus_ops =
+{
+    .pcie_enumerate    =   qemu_pci_enumerate,
+    .pci_cfg_write     =   qemu_pci_cfg_write,
+    .pci_cfg_read      =   qemu_pci_cfg_read,
+    .pci_map_bar       =   qemu_pci_map_bar,
+    .pci_map_bar64     =   qemu_pci_map_bar64,
+    .pci_msix_register = qemu_pci_msix_register,
+    .pci_msi_register  = qemu_pci_msi_register,
+};
+
+struct pcie_bus_s qemu_pcie_bus =
+{
+    .ops = &qemu_pcie_bus_ops,
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: qemu_pci_enumerate
+ *
+ * Description:
+ *  Scan the PCI bus and enumerate the devices.
+ *  Initialize any recognized devices, given in types.
+ *
+ * Input Parameters:
+ *   bus    - PCI-E bus structure
+ *   type   - List of pointers to devices types recognized, NULL terminated
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+

Review comment:
       if this function is for enumerating, I think it is not need to get the devices from BIOS/EFI, it can scan all of possible bdf and get the exist device.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420652375



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
##########
@@ -0,0 +1,229 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The PCI-E Definitions and part of the access routines are taken from
+ * Jailhouse inmate library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+#define __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include <nuttx/board.h>
+#include <nuttx/serial/uart_16550.h>
+#include <arch/board/board.h>
+
+#include "up_arch.h"
+#include "up_internal.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define PCI_REG_ADDR_PORT       0xcf8
+#define PCI_REG_DATA_PORT       0xcfc
+
+#define PCI_CONE                (1 << 31)
+
+/****************************************************************************
+ * Name: __qemu_pci_cfg_write
+ *
+ * Description:
+ *  Write 8, 16, 32 bits data to PCI-E configuration space of device
+ *  specified by dev
+ *
+ * Input Parameters:
+ *   bfd    - Device private data
+ *   buffer - A pointer to the read-only buffer of data to be written
+ *   size   - The number of bytes to send from the buffer
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static inline int __qemu_pci_cfg_write(uint16_t bfd, uintptr_t addr,
+                                       FAR const void *buffer,
+                                       unsigned int size)
+{

Review comment:
       this is X86 IO mode, yes? I think we'd better to judge the buffer whether is NULL.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420644572



##########
File path: drivers/pcie/pcie_root.c
##########
@@ -0,0 +1,358 @@
+/****************************************************************************
+ * nuttx/drivers/pcie/pcie_root.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/pcie/pcie.h>
+#include <nuttx/virt/qemu_pci.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_dev_type_s *pci_device_types[] =
+{
+#ifdef CONFIG_VIRT_QEMU_PCI_TEST
+  &pcie_type_qemu_pci_test,
+#endif /* CONFIG_VIRT_QEMU_PCI_TEST */
+  NULL,
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pcie_initialize
+ *
+ * Description:
+ *  Initialize the PCI-E bus and enumerate the devices with give devices
+ *  type array
+ *
+ * Input Parameters:
+ *   bus    - An PCIE bus
+ *   types  - A array of PCIE device types
+ *   num    - Number of device types
+ *
+ * Returned Value:
+ *   OK if the driver was successfully register; A negated errno value is
+ *   returned on any failure.
+ *
+ ****************************************************************************/
+
+int pcie_initialize(FAR struct pcie_bus_s *bus)
+{
+  return bus->ops->pcie_enumerate(bus, pci_device_types);
+}
+
+/****************************************************************************
+ * Name: pci_enable_device
+ *
+ * Description:
+ *  Enable device with flags
+ *
+ * Input Parameters:
+ *   bdf - device BDF
+ *   flags - device ability to be enabled
+ *
+ * Return value:
+ *   -EINVAL: error
+ *   OK: OK
+ *
+ ****************************************************************************/
+
+int pci_enable_device(FAR struct pcie_dev_s *dev, uint32_t flags)
+{

Review comment:
       I think we don't need the flags parameter, as my understand, enable PCIe device is do the MEM access enable, so set this bit in this function is OK.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420646115



##########
File path: drivers/pcie/pcie_root.c
##########
@@ -0,0 +1,358 @@
+/****************************************************************************
+ * nuttx/drivers/pcie/pcie_root.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/pcie/pcie.h>
+#include <nuttx/virt/qemu_pci.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_dev_type_s *pci_device_types[] =
+{
+#ifdef CONFIG_VIRT_QEMU_PCI_TEST
+  &pcie_type_qemu_pci_test,
+#endif /* CONFIG_VIRT_QEMU_PCI_TEST */
+  NULL,
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pcie_initialize
+ *
+ * Description:
+ *  Initialize the PCI-E bus and enumerate the devices with give devices
+ *  type array
+ *
+ * Input Parameters:
+ *   bus    - An PCIE bus
+ *   types  - A array of PCIE device types
+ *   num    - Number of device types
+ *
+ * Returned Value:
+ *   OK if the driver was successfully register; A negated errno value is
+ *   returned on any failure.
+ *
+ ****************************************************************************/
+
+int pcie_initialize(FAR struct pcie_bus_s *bus)
+{
+  return bus->ops->pcie_enumerate(bus, pci_device_types);
+}
+
+/****************************************************************************
+ * Name: pci_enable_device
+ *
+ * Description:
+ *  Enable device with flags
+ *
+ * Input Parameters:
+ *   bdf - device BDF
+ *   flags - device ability to be enabled
+ *
+ * Return value:
+ *   -EINVAL: error
+ *   OK: OK
+ *
+ ****************************************************************************/
+
+int pci_enable_device(FAR struct pcie_dev_s *dev, uint32_t flags)
+{
+  uint16_t old_cmd;
+  uint16_t cmd;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_COMMAND, &old_cmd, 2);
+
+  cmd = old_cmd | flags;
+
+  dev->bus->ops->pci_cfg_write(dev, PCI_CFG_COMMAND, &cmd, 2);
+
+  pciinfo("%02x:%02x.%x, CMD: %x -> %x\n",
+          dev->bdf >> 8, (dev->bdf >> 3) & 0x1f, dev->bdf & 0x3,
+          old_cmd, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_find_cap
+ *
+ * Description:
+ *  Search through the PCI-e device capability list to find given capability.
+ *
+ * Input Parameters:
+ *   dev - Device
+ *   cap - Bitmask of capability
+ *
+ * Returned Value:
+ *   -1: Capability not supported
+ *   other: the offset in PCI configuration space to the capability structure
+ *
+ ****************************************************************************/
+
+int pci_find_cap(FAR struct pcie_dev_s *dev, uint16_t cap)
+{
+  uint8_t pos = PCI_CFG_CAP_PTR - 1;
+  uint16_t status;
+  uint8_t rcap;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_STATUS, &status, 2);
+
+  if (!(status & PCI_STS_CAPS))
+      return -EINVAL;
+
+  while (1)
+    {
+      dev->bus->ops->pci_cfg_read(dev, pos + 1, &pos, 1);
+      if (pos == 0)
+          return -EINVAL;
+
+      dev->bus->ops->pci_cfg_read(dev, pos, &rcap, 1);
+
+      if (rcap == cap)
+          return pos;
+    }
+}
+
+/****************************************************************************
+ * Name: pci_get_bar
+ *
+ * Description:
+ *  Get a 32 bits bar
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   ret    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_get_bar(FAR struct pcie_dev_s *dev, uint32_t bar,
+                uint32_t *ret)
+{
+  if (bar > 5)
+      return -EINVAL;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_BAR + bar * 4, ret, 4);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_get_bar64
+ *
+ * Description:
+ *  Get a 64 bits bar
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   ret    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_get_bar64(FAR struct pcie_dev_s *dev, uint32_t bar,
+                  uint64_t *ret)
+{
+  if (bar > 5 || ((bar % 2) != 0))
+      return -EINVAL;

Review comment:
       if the device have 64 bit BAR, the BAR number will never more than 5




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420648865



##########
File path: drivers/pcie/pcie_root.c
##########
@@ -0,0 +1,358 @@
+/****************************************************************************
+ * nuttx/drivers/pcie/pcie_root.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/pcie/pcie.h>
+#include <nuttx/virt/qemu_pci.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_dev_type_s *pci_device_types[] =
+{
+#ifdef CONFIG_VIRT_QEMU_PCI_TEST
+  &pcie_type_qemu_pci_test,
+#endif /* CONFIG_VIRT_QEMU_PCI_TEST */
+  NULL,
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pcie_initialize
+ *
+ * Description:
+ *  Initialize the PCI-E bus and enumerate the devices with give devices
+ *  type array
+ *
+ * Input Parameters:
+ *   bus    - An PCIE bus
+ *   types  - A array of PCIE device types
+ *   num    - Number of device types
+ *
+ * Returned Value:
+ *   OK if the driver was successfully register; A negated errno value is
+ *   returned on any failure.
+ *
+ ****************************************************************************/
+
+int pcie_initialize(FAR struct pcie_bus_s *bus)
+{
+  return bus->ops->pcie_enumerate(bus, pci_device_types);
+}
+
+/****************************************************************************
+ * Name: pci_enable_device
+ *
+ * Description:
+ *  Enable device with flags
+ *
+ * Input Parameters:
+ *   bdf - device BDF
+ *   flags - device ability to be enabled
+ *
+ * Return value:
+ *   -EINVAL: error
+ *   OK: OK
+ *
+ ****************************************************************************/
+
+int pci_enable_device(FAR struct pcie_dev_s *dev, uint32_t flags)
+{

Review comment:
       I agree, patching now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] btashton commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-624227298


   @sonicyang are the qemu_pci* files the only ones that came from jailroot?
   
   I have some PCI code I wrote in the past that maybe could serve this purpose and avoid the license questions. We already need to make a fair amount of changes for code style. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r421179067



##########
File path: drivers/pcie/pcie_root.c
##########
@@ -0,0 +1,446 @@
+/****************************************************************************
+ * nuttx/drivers/pcie/pcie_root.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/pcie/pcie.h>
+#include <nuttx/virt/qemu_pci.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_dev_type_s *pci_device_types[] =
+{
+#ifdef CONFIG_VIRT_QEMU_PCI_TEST
+  &pcie_type_qemu_pci_test,
+#endif /* CONFIG_VIRT_QEMU_PCI_TEST */
+  NULL,
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pci_enumerate
+ *
+ * Description:
+ *  Scan the PCI bus and enumerate the devices.
+ *  Initialize any recognized devices, given in types.
+ *
+ * Input Parameters:
+ *   bus    - PCI-E bus structure
+ *   type   - List of pointers to devices types recognized, NULL terminated
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_enumerate(FAR struct pcie_bus_s *bus,
+                  FAR struct pcie_dev_type_s **types)
+{
+  unsigned int bdf;
+  uint16_t vid;
+  uint16_t id;
+  uint16_t rev;
+  struct pcie_dev_s tmp_dev;
+  struct pcie_dev_type_s tmp_type =
+    {
+      .name = "Unknown",
+      .vendor = PCI_ID_ANY,
+      .device = PCI_ID_ANY,
+      .class_rev = PCI_ID_ANY,
+      .probe = NULL,
+    };
+
+  if (!bus)
+      return -EINVAL;
+  if (!types)
+      return -EINVAL;
+
+  for (bdf = 0; bdf < CONFIG_PCIE_MAX_BDF; bdf++)
+    {
+      tmp_dev.bus = bus;
+      tmp_dev.type = &tmp_type;
+      tmp_dev.bdf = bdf;
+
+      bus->ops->pci_cfg_read(&tmp_dev, PCI_CFG_VENDOR_ID, &vid, 2);
+      bus->ops->pci_cfg_read(&tmp_dev, PCI_CFG_DEVICE_ID, &id, 2);
+      bus->ops->pci_cfg_read(&tmp_dev, PCI_CFG_REVERSION, &rev, 2);
+
+      if (vid == PCI_ID_ANY)
+        continue;
+
+      pciinfo("[%02x:%02x.%x] Found %04x:%04x, class/reversion %08x\n",
+              bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3,
+              vid, id, rev);
+
+      for (int i = 0; types[i] != NULL; i++)
+        {
+          if (types[i]->vendor == PCI_ID_ANY ||
+              types[i]->vendor == vid)
+            {
+              if (types[i]->device == PCI_ID_ANY ||
+                  types[i]->device == id)
+                {
+                  if (types[i]->class_rev == PCI_ID_ANY ||
+                      types[i]->class_rev == rev)
+                    {
+                      if (types[i]->probe)
+                        {
+                          pciinfo("[%02x:%02x.%x] %s\n",
+                                  bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3,
+                                  types[i]->name);
+                          types[i]->probe(bus, types[i], bdf);
+                        }
+                      else
+                        {
+                          pcierr("[%02x:%02x.%x] Error: Invalid \
+                                  device probe function\n",
+                                  bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3);
+                        }
+                      break;
+                    }
+                }
+            }
+        }
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pcie_initialize
+ *
+ * Description:
+ *  Initialize the PCI-E bus and enumerate the devices with give devices
+ *  type array
+ *
+ * Input Parameters:
+ *   bus    - An PCIE bus
+ *   types  - A array of PCIE device types
+ *   num    - Number of device types
+ *
+ * Returned Value:
+ *   OK if the driver was successfully register; A negated errno value is
+ *   returned on any failure.
+ *
+ ****************************************************************************/
+
+int pcie_initialize(FAR struct pcie_bus_s *bus)
+{
+  return pci_enumerate(bus, pci_device_types);
+}
+
+/****************************************************************************
+ * Name: pci_enable_device
+ *
+ * Description:
+ *  Enable device with MMIO
+ *
+ * Input Parameters:
+ *   dev - device
+ *
+ * Return value:
+ *   -EINVAL: error
+ *   OK: OK
+ *
+ ****************************************************************************/
+
+int pci_enable_device(FAR struct pcie_dev_s *dev)
+{
+  uint16_t old_cmd;
+  uint16_t cmd;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_COMMAND, &old_cmd, 2);
+
+  cmd = old_cmd | (PCI_CMD_MASTER | PCI_CMD_MEM);
+
+  dev->bus->ops->pci_cfg_write(dev, PCI_CFG_COMMAND, &cmd, 2);
+
+  pciinfo("%02x:%02x.%x, CMD: %x -> %x\n",
+          dev->bdf >> 8, (dev->bdf >> 3) & 0x1f, dev->bdf & 0x3,
+          old_cmd, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_find_cap
+ *
+ * Description:
+ *  Search through the PCI-e device capability list to find given capability.
+ *
+ * Input Parameters:
+ *   dev - Device
+ *   cap - Bitmask of capability
+ *
+ * Returned Value:
+ *   -1: Capability not supported
+ *   other: the offset in PCI configuration space to the capability structure
+ *
+ ****************************************************************************/
+
+int pci_find_cap(FAR struct pcie_dev_s *dev, uint16_t cap)
+{
+  uint8_t pos = PCI_CFG_CAP_PTR - 1;
+  uint16_t status;
+  uint8_t rcap;
+

Review comment:
       e.g.
   if (dev->bus->ops->pci_cfg_read == NULL)
             return -EINVAL;
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625881521


   @btashton Thank you.
   Let's continue there.
   
   > I also have a PCIe FPGA board that should be here in a few weeks that will allow me to test some of this against a RISC-V softcore.
   
   It's good that we can test it on some other architectures.
   Looking forward to it.
   Personally, I only have a Intel Xeon machine and a R-Pi 4 at the moment.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] btashton commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625159985


   @sonicyang I did some work here https://github.com/btashton/incubator-nuttx/tree/pci-btashton
   I have ported over a good chunk of the code that I wrote before in place of the Jailhouse code.  I have not pulled in any of my msi/msix code.  I did change the logic around enumeration to support bridges as well as be smarter about traversing the space.
   
   https://github.com/btashton/incubator-nuttx/tree/pci-btashton
   Let me know what you think and I can keep porting over more of the logic if it makes sense.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420647360



##########
File path: drivers/pcie/pcie_root.c
##########
@@ -0,0 +1,358 @@
+/****************************************************************************
+ * nuttx/drivers/pcie/pcie_root.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/pcie/pcie.h>
+#include <nuttx/virt/qemu_pci.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_dev_type_s *pci_device_types[] =
+{
+#ifdef CONFIG_VIRT_QEMU_PCI_TEST
+  &pcie_type_qemu_pci_test,
+#endif /* CONFIG_VIRT_QEMU_PCI_TEST */
+  NULL,
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: pcie_initialize
+ *
+ * Description:
+ *  Initialize the PCI-E bus and enumerate the devices with give devices
+ *  type array
+ *
+ * Input Parameters:
+ *   bus    - An PCIE bus
+ *   types  - A array of PCIE device types
+ *   num    - Number of device types
+ *
+ * Returned Value:
+ *   OK if the driver was successfully register; A negated errno value is
+ *   returned on any failure.
+ *
+ ****************************************************************************/
+
+int pcie_initialize(FAR struct pcie_bus_s *bus)
+{
+  return bus->ops->pcie_enumerate(bus, pci_device_types);
+}
+
+/****************************************************************************
+ * Name: pci_enable_device
+ *
+ * Description:
+ *  Enable device with flags
+ *
+ * Input Parameters:
+ *   bdf - device BDF
+ *   flags - device ability to be enabled
+ *
+ * Return value:
+ *   -EINVAL: error
+ *   OK: OK
+ *
+ ****************************************************************************/
+
+int pci_enable_device(FAR struct pcie_dev_s *dev, uint32_t flags)
+{
+  uint16_t old_cmd;
+  uint16_t cmd;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_COMMAND, &old_cmd, 2);
+
+  cmd = old_cmd | flags;
+
+  dev->bus->ops->pci_cfg_write(dev, PCI_CFG_COMMAND, &cmd, 2);
+
+  pciinfo("%02x:%02x.%x, CMD: %x -> %x\n",
+          dev->bdf >> 8, (dev->bdf >> 3) & 0x1f, dev->bdf & 0x3,
+          old_cmd, cmd);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_find_cap
+ *
+ * Description:
+ *  Search through the PCI-e device capability list to find given capability.
+ *
+ * Input Parameters:
+ *   dev - Device
+ *   cap - Bitmask of capability
+ *
+ * Returned Value:
+ *   -1: Capability not supported
+ *   other: the offset in PCI configuration space to the capability structure
+ *
+ ****************************************************************************/
+
+int pci_find_cap(FAR struct pcie_dev_s *dev, uint16_t cap)
+{
+  uint8_t pos = PCI_CFG_CAP_PTR - 1;
+  uint16_t status;
+  uint8_t rcap;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_STATUS, &status, 2);
+
+  if (!(status & PCI_STS_CAPS))
+      return -EINVAL;
+
+  while (1)
+    {
+      dev->bus->ops->pci_cfg_read(dev, pos + 1, &pos, 1);
+      if (pos == 0)
+          return -EINVAL;
+
+      dev->bus->ops->pci_cfg_read(dev, pos, &rcap, 1);
+
+      if (rcap == cap)
+          return pos;
+    }
+}
+
+/****************************************************************************
+ * Name: pci_get_bar
+ *
+ * Description:
+ *  Get a 32 bits bar
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   ret    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_get_bar(FAR struct pcie_dev_s *dev, uint32_t bar,
+                uint32_t *ret)
+{
+  if (bar > 5)
+      return -EINVAL;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_BAR + bar * 4, ret, 4);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_get_bar64
+ *
+ * Description:
+ *  Get a 64 bits bar
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   ret    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_get_bar64(FAR struct pcie_dev_s *dev, uint32_t bar,
+                  uint64_t *ret)
+{
+  if (bar > 5 || ((bar % 2) != 0))
+      return -EINVAL;
+
+  uint32_t barmem1;
+  uint32_t barmem2;
+
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_BAR + bar * 4, &barmem1, 4);
+  dev->bus->ops->pci_cfg_read(dev, PCI_CFG_BAR + bar * 4 + 4, &barmem2, 4);
+
+  *ret = ((uint64_t)barmem2 << 32) | barmem1;
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_set_bar
+ *
+ * Description:
+ *  Set a 32 bits bar
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   val    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_set_bar(FAR struct pcie_dev_s *dev, uint32_t bar,
+                uint32_t val)
+{
+  if (bar > 5)
+      return -EINVAL;
+
+  dev->bus->ops->pci_cfg_write(dev, PCI_CFG_BAR + bar * 4, &val, 4);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: pci_set_bar64
+ *
+ * Description:
+ *  Set a 64 bits bar
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   val    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+int pci_set_bar64(FAR struct pcie_dev_s *dev, uint32_t bar,
+                  uint64_t val)
+{
+  if (bar > 5 || ((bar % 2) != 0))
+      return -EINVAL;
+

Review comment:
       same as pci_get_bar64 function




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420713285



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
##########
@@ -0,0 +1,467 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The MSI and MSI-X vector setup function are taken from Jailhouse inmate
+ * library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include "qemu_pcie_readwrite.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define QEMU_PCIE_MAX_BDF 0x10000
+
+/****************************************************************************
+ * Private Functions Definitions
+ ****************************************************************************/
+
+static int qemu_pci_enumerate(FAR struct pcie_bus_s *bus,
+                               FAR struct pcie_dev_type_s **types);
+
+static int qemu_pci_cfg_write(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                              FAR const void *buffer, unsigned int size);
+
+static int qemu_pci_cfg_read(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                             FAR void *buffer, unsigned int size);
+
+static int qemu_pci_map_bar(FAR struct pcie_dev_s *dev, uint32_t addr,
+                            unsigned long length);
+
+static int qemu_pci_map_bar64(FAR struct pcie_dev_s *dev, uint64_t addr,
+                              unsigned long length);
+
+static int qemu_pci_msix_register(FAR struct pcie_dev_s *dev,
+                                  uint32_t vector, uint32_t index);
+
+static int qemu_pci_msi_register(FAR struct pcie_dev_s *dev,
+                                 uint16_t vector);
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_bus_ops_s qemu_pcie_bus_ops =
+{
+    .pcie_enumerate    =   qemu_pci_enumerate,
+    .pci_cfg_write     =   qemu_pci_cfg_write,
+    .pci_cfg_read      =   qemu_pci_cfg_read,
+    .pci_map_bar       =   qemu_pci_map_bar,
+    .pci_map_bar64     =   qemu_pci_map_bar64,
+    .pci_msix_register = qemu_pci_msix_register,
+    .pci_msi_register  = qemu_pci_msi_register,
+};
+
+struct pcie_bus_s qemu_pcie_bus =
+{
+    .ops = &qemu_pcie_bus_ops,
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: qemu_pci_enumerate
+ *
+ * Description:
+ *  Scan the PCI bus and enumerate the devices.
+ *  Initialize any recognized devices, given in types.
+ *
+ * Input Parameters:
+ *   bus    - PCI-E bus structure
+ *   type   - List of pointers to devices types recognized, NULL terminated
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static int qemu_pci_enumerate(FAR struct pcie_bus_s *bus,
+                               FAR struct pcie_dev_type_s **types)
+{
+  unsigned int bdf;
+  uint16_t vid;
+  uint16_t id;
+  uint16_t rev;
+
+  if (!bus)
+      return -EINVAL;
+  if (!types)
+      return -EINVAL;
+
+  for (bdf = 0; bdf < QEMU_PCIE_MAX_BDF; bdf++)
+    {
+      __qemu_pci_cfg_read(bdf, PCI_CFG_VENDOR_ID, &vid, 2);
+      __qemu_pci_cfg_read(bdf, PCI_CFG_DEVICE_ID, &id, 2);
+      __qemu_pci_cfg_read(bdf, PCI_CFG_REVERSION, &rev, 2);
+
+      if (vid == PCI_ID_ANY)
+        continue;
+
+      pciinfo("[%02x:%02x.%x] Found %04x:%04x, class/reversion %08x\n",
+              bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3,
+              vid, id, rev);
+
+      for (int i = 0; types[i] != NULL; i++)
+        {
+          if (types[i]->vendor == PCI_ID_ANY ||
+              types[i]->vendor == vid)
+            {
+              if (types[i]->device == PCI_ID_ANY ||
+                  types[i]->device == id)
+                {
+                  if (types[i]->class_rev == PCI_ID_ANY ||
+                      types[i]->class_rev == rev)
+                    {
+                      if (types[i]->probe)
+                        {
+                          pciinfo("[%02x:%02x.%x] %s\n",
+                                  bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3,
+                                  types[i]->name);
+                          types[i]->probe(bus, types[i], bdf);
+                        }
+                      else
+                        {
+                          pcierr("[%02x:%02x.%x] Error: Invalid \
+                                  device probe function\n",
+                                  bdf >> 8, (bdf >> 3) & 0x1f, bdf & 0x3);
+                        }
+                      break;
+                    }
+                }
+            }
+        }
+    }
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: qemu_pci_cfg_write
+ *
+ * Description:
+ *  Write 8, 16, 32, 64 bits data to PCI-E configuration space of device
+ *  specified by dev
+ *
+ * Input Parameters:
+ *   bdf    - Device private data
+ *   buffer - A pointer to the read-only buffer of data to be written
+ *   size   - The number of bytes to send from the buffer
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static int qemu_pci_cfg_write(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                              FAR const void *buffer, unsigned int size)
+{
+  if(!buffer)
+      return -EINVAL;
+
+  switch (size)
+    {
+      case 1:
+      case 2:
+      case 4:
+        return __qemu_pci_cfg_write(dev->bdf, addr, buffer, size);
+      case 8:
+        return __qemu_pci_cfg_write(dev->bdf, addr, buffer, size);
+      default:
+        return -EINVAL;
+    }
+}
+
+/****************************************************************************
+ * Name: qemu_pci_cfg_read
+ *
+ * Description:
+ *  Read 8, 16, 32, 64 bits data from PCI-E configuration space of device
+ *  specified by dev
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   buffer - A pointer to a buffer to receive the data from the device
+ *   size   - The requested number of bytes to be read
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static int qemu_pci_cfg_read(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                             FAR void *buffer, unsigned int size)
+{
+  if(!buffer)
+      return -EINVAL;
+
+  switch (size)
+    {
+      case 1:
+      case 2:
+      case 4:
+        return __qemu_pci_cfg_read(dev->bdf, addr, buffer, size);
+      case 8:
+        return __qemu_pci_cfg_read64(dev->bdf, addr, buffer, size);
+      default:
+        return -EINVAL;
+    }
+}
+
+/****************************************************************************
+ * Name: qemu_pci_map_bar
+ *
+ * Description:
+ *  Map address in a 32 bits bar in the memory address space
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   length - Map length, multiple of PAGE_SIZE
+ *   ret    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static int qemu_pci_map_bar(FAR struct pcie_dev_s *dev, uint32_t addr,
+                            unsigned long length)
+{
+  up_map_region((void *)((uintptr_t)addr), length,
+      X86_PAGE_WR | X86_PAGE_PRESENT | X86_PAGE_NOCACHE | X86_PAGE_GLOBAL);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: qemu_pci_map_bar64
+ *
+ * Description:
+ *  Map address in a 64 bits bar in the memory address space
+ *
+ * Input Parameters:
+ *   dev    - Device private data
+ *   bar    - Bar number
+ *   length - Map length, multiple of PAGE_SIZE
+ *   ret    - Bar Content
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static int qemu_pci_map_bar64(FAR struct pcie_dev_s *dev, uint64_t addr,
+                              unsigned long length)
+{
+  up_map_region((void *)((uintptr_t)addr), length,
+      X86_PAGE_WR | X86_PAGE_PRESENT | X86_PAGE_NOCACHE | X86_PAGE_GLOBAL);
+
+  return OK;
+}
+
+/****************************************************************************
+ * Name: qemu_pci_msix_register
+ *
+ * Description:
+ *  Map a device MSI-X vector to a platform IRQ vector
+ *
+ * Input Parameters:
+ *   dev - Device
+ *   vector - IRQ number of the platform
+ *   index  - Device MSI-X vector number
+ *
+ * Returned Value:
+ *   <0: Mapping failed
+ *    0: Mapping succeed
+ *
+ ****************************************************************************/
+
+static int qemu_pci_msix_register(FAR struct pcie_dev_s *dev,
+                                  uint32_t vector, uint32_t index)
+{
+  unsigned int bar;
+  uint16_t message_control;
+  uint32_t table_bar_ind;
+  uint32_t lo_table_addr;
+  uint32_t hi_table_addr;
+  uint64_t msix_table_addr = 0;
+
+  int cap = pci_find_cap(dev, PCI_CAP_MSIX);
+  if (cap < 0)
+      return -EINVAL;
+

Review comment:
       we'd better define some macros instead of 2 4.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang edited a comment on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang edited a comment on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625793332


   @btashton I have checked your modifications.
   I didn't make it work with my serial card because of lacking find_cap and msi/msix routines.
   But I did confirmed that it did scan the bus without crashing my Intel Xeon machine.
   
   I did noticed this "smart" way of scanning bus didn't work with Jailhouse, because Jailhouse didn't have a full PCI hierarchy, e.g. root device, etc. Devices are passed to the cells via VT-d separately.
   
   I am currently working on ivshmem device driver for Jailhouse, I hope we can take this problem into account.
   
   I purpose that we should open a PCI feature branch, cooperate there with PR.
   Of course, we need some one with permission to do this.
   
   What do you think?
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625326953


   @btashton It sounds good to me.
   
   Should we support 64bit r/w natively? 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625203136


   Seems quite good.
   I will try it out on my jailhuose machine with a serial PCI-E card on weekend.
   Hope it works.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] btashton edited a comment on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
btashton edited a comment on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-624247753


   > Did the PR checks fail to start for this one too? There was a rumor that if you rebase then force push the branch, the checks will start. Can you try that?
   
   @patacongo :
   Seems like if there is any issues on github's side you get stuck having to re push to trigger https://github.community/t5/How-to-use-Git-and-GitHub/Is-it-possible-to-manually-force-an-action-workflow-to-be-re-run/m-p/54996/highlight/true#M11604


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-624415804


   @patacongo  @btashton 
   I will try to force push it, but where should I rebase to?
   
   About the licensing problem, if we have another implementation, it will be great.
   Let's first find a way to cooperate.
   
   The affected files are the  `driver/pcie` and `board/` files.
   Not all of them were copied, though. I tried to rewrite most of them based on the same logic.
   I should be able to identify those and make a list.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] btashton commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-625844424


   @sonicyang -- First off sorry I clicked edit on your comment instead of reply, should have restored it back to what you had.
   
   > @btashton I have checked your modifications.
   > I didn't make it work with my serial card because of lacking find_cap and msi/msix routines.
   
   This is to be expected. I am reworking those right now, but should have that done this weekend.
   
   > But I did confirmed that it did scan the bus without crashing my Intel Xeon machine.
   
   Great! I have only been messing with this via QEMU partially because it lets be create custom bus hierarchies.
   
   > 
   > I did noticed this "smart" way of scanning bus didn't work with Jailhouse, because Jailhouse didn't have a full PCI hierarchy, e.g. root device, etc. Devices are passed to the cells via VT-d separately.
   
   This should not be a problem, but I did see that Jailhouse breaks the PCI spec in a way I did not check for. They let you pass individual device functions which means that when I check func 0 I will stop looking. I bet this is where things went wrong. I added a patch to work around this to my branch. I it would be great if you could see if it finds your devices.
   
   > 
   > I am currently working on ivshmem device driver for Jailhouse, I hope we can take this problem into account.
   
   We certainly should handle this case. I made a note in my patch about how to detect Jailhouse to work around this issue (for now I hard coded it on)
   
   > 
   > I purpose that we should open a PCI feature branch, cooperate there with PR.
   > Of course, we need some one with permission to do this.
   > 
   
   I created the branch off of what was in this PR called pci. https://github.com/apache/incubator-nuttx/tree/pci
   I think this is a great starting point and we can review it more as a whole once we have it more mature. I also don't want to block you while I am working on some more of the low level primitives without the jailroot code. I also have a PCIe FPGA board that should be here in a few weeks that will allow me to test some of this against a RISC-V softcore.
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420708159



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
##########
@@ -0,0 +1,229 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The PCI-E Definitions and part of the access routines are taken from
+ * Jailhouse inmate library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+#define __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include <nuttx/board.h>
+#include <nuttx/serial/uart_16550.h>
+#include <arch/board/board.h>
+
+#include "up_arch.h"
+#include "up_internal.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define PCI_REG_ADDR_PORT       0xcf8
+#define PCI_REG_DATA_PORT       0xcfc
+
+#define PCI_CONE                (1 << 31)
+
+/****************************************************************************
+ * Name: __qemu_pci_cfg_write
+ *
+ * Description:
+ *  Write 8, 16, 32 bits data to PCI-E configuration space of device
+ *  specified by dev
+ *
+ * Input Parameters:
+ *   bfd    - Device private data
+ *   buffer - A pointer to the read-only buffer of data to be written
+ *   size   - The number of bytes to send from the buffer
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static inline int __qemu_pci_cfg_write(uint16_t bfd, uintptr_t addr,
+                                       FAR const void *buffer,
+                                       unsigned int size)
+{

Review comment:
       That's a good idea, will do.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420663754



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
##########
@@ -0,0 +1,461 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The MSI and MSI-X vector setup function are taken from Jailhouse inmate
+ * library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include "qemu_pcie_readwrite.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define QEMU_PCIE_MAX_BDF 0x10000
+
+/****************************************************************************
+ * Private Functions Definitions
+ ****************************************************************************/
+
+static int qemu_pci_enumerate(FAR struct pcie_bus_s *bus,
+                               FAR struct pcie_dev_type_s **types);
+
+static int qemu_pci_cfg_write(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                              FAR const void *buffer, unsigned int size);
+
+static int qemu_pci_cfg_read(FAR struct pcie_dev_s *dev, uintptr_t addr,
+                             FAR void *buffer, unsigned int size);
+
+static int qemu_pci_map_bar(FAR struct pcie_dev_s *dev, uint32_t addr,
+                            unsigned long length);
+
+static int qemu_pci_map_bar64(FAR struct pcie_dev_s *dev, uint64_t addr,
+                              unsigned long length);
+
+static int qemu_pci_msix_register(FAR struct pcie_dev_s *dev,
+                                  uint32_t vector, uint32_t index);
+
+static int qemu_pci_msi_register(FAR struct pcie_dev_s *dev,
+                                 uint16_t vector);
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+struct pcie_bus_ops_s qemu_pcie_bus_ops =
+{
+    .pcie_enumerate    =   qemu_pci_enumerate,
+    .pci_cfg_write     =   qemu_pci_cfg_write,
+    .pci_cfg_read      =   qemu_pci_cfg_read,
+    .pci_map_bar       =   qemu_pci_map_bar,
+    .pci_map_bar64     =   qemu_pci_map_bar64,
+    .pci_msix_register = qemu_pci_msix_register,
+    .pci_msi_register  = qemu_pci_msi_register,
+};
+
+struct pcie_bus_s qemu_pcie_bus =
+{
+    .ops = &qemu_pcie_bus_ops,
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: qemu_pci_enumerate
+ *
+ * Description:
+ *  Scan the PCI bus and enumerate the devices.
+ *  Initialize any recognized devices, given in types.
+ *
+ * Input Parameters:
+ *   bus    - PCI-E bus structure
+ *   type   - List of pointers to devices types recognized, NULL terminated
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+

Review comment:
       the enumerate function is common code, I think move this function to drivers/pcie is better.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420674458



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
##########
@@ -0,0 +1,229 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The PCI-E Definitions and part of the access routines are taken from
+ * Jailhouse inmate library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+#define __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include <nuttx/board.h>
+#include <nuttx/serial/uart_16550.h>
+#include <arch/board/board.h>
+
+#include "up_arch.h"
+#include "up_internal.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define PCI_REG_ADDR_PORT       0xcf8
+#define PCI_REG_DATA_PORT       0xcfc
+
+#define PCI_CONE                (1 << 31)
+
+/****************************************************************************
+ * Name: __qemu_pci_cfg_write
+ *
+ * Description:
+ *  Write 8, 16, 32 bits data to PCI-E configuration space of device
+ *  specified by dev
+ *
+ * Input Parameters:
+ *   bfd    - Device private data
+ *   buffer - A pointer to the read-only buffer of data to be written
+ *   size   - The number of bytes to send from the buffer
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static inline int __qemu_pci_cfg_write(uint16_t bfd, uintptr_t addr,
+                                       FAR const void *buffer,
+                                       unsigned int size)
+{

Review comment:
       What do you mean IO mode?
   
   Is it IO mode access to PCI configuration space? If so, then yes.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] baoxiaowei-xiaomi commented on a change in pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
baoxiaowei-xiaomi commented on a change in pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#discussion_r420652375



##########
File path: boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
##########
@@ -0,0 +1,229 @@
+/****************************************************************************
+ * boards/x86_64/intel64/qemu-intel64/src/qemu_pcie_readwrite.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The PCI-E Definitions and part of the access routines are taken from
+ * Jailhouse inmate library
+ *
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) Siemens AG, 2014
+ *
+ * Authors:
+ *  Jan Kiszka <ja...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Alternatively, you can use or redistribute this file under the following
+ * BSD license:
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+#define __INCLUDE_NUTTX_PCIE_PCIE_READWRITE_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+
+#include <nuttx/pcie/pcie.h>
+
+#include <nuttx/board.h>
+#include <nuttx/serial/uart_16550.h>
+#include <arch/board/board.h>
+
+#include "up_arch.h"
+#include "up_internal.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define PCI_REG_ADDR_PORT       0xcf8
+#define PCI_REG_DATA_PORT       0xcfc
+
+#define PCI_CONE                (1 << 31)
+
+/****************************************************************************
+ * Name: __qemu_pci_cfg_write
+ *
+ * Description:
+ *  Write 8, 16, 32 bits data to PCI-E configuration space of device
+ *  specified by dev
+ *
+ * Input Parameters:
+ *   bfd    - Device private data
+ *   buffer - A pointer to the read-only buffer of data to be written
+ *   size   - The number of bytes to send from the buffer
+ *
+ * Returned Value:
+ *   0: success, <0: A negated errno
+ *
+ ****************************************************************************/
+
+static inline int __qemu_pci_cfg_write(uint16_t bfd, uintptr_t addr,
+                                       FAR const void *buffer,
+                                       unsigned int size)
+{

Review comment:
       this is X86 IO mode, yes?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] sonicyang commented on pull request #979: PCI-E support with x86_64/qemu example

Posted by GitBox <gi...@apache.org>.
sonicyang commented on pull request #979:
URL: https://github.com/apache/incubator-nuttx/pull/979#issuecomment-624588711


   I need suggestion on this, should we use pci or pcie as the function prefix?
   
   I am kind of mixing these 2 at the moment, but I want to unify it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org