[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC-PATCH V2] hw/pci/pci_example.c : Added a new pci e

From: Marcel Apfelbaum
Subject: Re: [Qemu-devel] [RFC-PATCH V2] hw/pci/pci_example.c : Added a new pci example device
Date: Sat, 22 Sep 2018 14:25:16 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

Hi Yoni, Cornelia,

On 09/20/2018 04:59 PM, Cornelia Huck wrote:
On Wed,  5 Sep 2018 12:38:00 +0300
Yoni Bettan <address@hidden> wrote:

The main goal is to add 2 example devices to be used as templates or guideline
for contributors when they wish to create a new device, the first is a PCI
device and the second will be a VirtIO device.

BTW, it seems we have another device in QEMU for educational
purposes, see hw/misc/edu.c.
Maybe we can "connect" them, or not.

Those devices may be compiled by default with qemu, to ensure they won't
break, but not have to be linked, so performance won't decrease.

Another reason for such devices is to document "the right way" to write
a new device in qemu.

Those 2 devices have the same functionality and are multiplying a
number by 2, for now only numbers in range [0:127] (result is in range [0:254])
are supported (1-byte size input\output).

* PCI device:
   - its driver is located at https://github.com/ybettan/QemuDeviceDrivers.git

I think we may add the driver to QEMU'S contrib directory, after all
is a documentation project, maybe is better to have it all in the same place.

* virtIO device:
   - still not done.
   - its driver isn't written yet but will be added to the same Github 
     written above.

Again, maybe we can have it in QEMU's contrib directory.

   - when this device is done the next step is to insert it into the virtio
In which way? You might want to reserve an id for 'testing purposes' or
somesuch, which should be easy to do right now.

I think we can "link" a virtio requirement to parts of the implementation.
This will make the virtio spec more approachable.

For example: "Interrupts are working like this ..., a possible implementation
would be <link to QEMU/contrib/file/line nr> "

I personally find very handy a demo project "explaining" a spec doc.

Both devices can be found at https://github.com/ybettan/qemu.git under 'pci'
and 'virtio' branches.

In addition I am writing a blog to give a logical overview of the virtio
protocol and a step-by-step guide to write a new virtio device.
This blog can be found at https://howtovms.wordpress.com.


Signed-off-by: Yoni Bettan <address@hidden>

Some questions I would like hear your opinion on:

1. How in your opinion can we make this device compile with qemu to ensure it
    won't break but without enabling it by default on production builds ?
Add CONFIG_EXAMPLE_DEVICES; distributions etc. can then disable it.


2. Do you thinks this device should also support MSI-X ?
If you want to provide a template that people can copy from, it
probably makes sense. But it certainly can be done in a later patch.

- Added an explanation on why should we have such devices in the commit message.
- Added some questions at the end of the patch according to Eduardo
   Habkost's review and Cornelia Huck's review in V1.
- Added comments on unclear parts.
- Removed unnecessary macros.
- Changed names to fit CODING_SYLE.

  hw/pci/Makefile.objs |   1 +
  hw/pci/pci_example.c | 327 +++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 328 insertions(+)
  create mode 100644 hw/pci/pci_example.c

diff --git a/hw/pci/pci_example.c b/hw/pci/pci_example.c
new file mode 100644
index 0000000000..ca510a2b3b
--- /dev/null
+++ b/hw/pci/pci_example.c
@@ -0,0 +1,327 @@
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+/* this will be the name of the device from qemu point of view */

+#define TYPE_PCI_EXAMPLE "pci-example"
+/* each device should have a macro to be cast to using OBJECT_CHECK macro */
+#define PCI_EXAMPLE_DEVICE(obj)  \
+    OBJECT_CHECK(PCIExampleDevice, (obj), TYPE_PCI_EXAMPLE)
+#define DMA_BUF_SIZE 4096
+/*                                 PCI Struct                                */
+typedef struct PCIExampleDevice {
+    /*< private >*/
+    /* this device inherits from PCIDevice according to QEMU Object Model */
+    PCIDevice parent_obj;
+    /*< public >*/
+    MemoryRegion portio;
+    MemoryRegion mmio;
+    MemoryRegion irqio;
+    MemoryRegion dmaio;

You may want to add a concise explanation of the above
fields for people new to QEMU/PCI world.

The same comment for all the code. Since is a documentation
project I think you should add as much info as possible.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]