Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
Provide the private MSI setup functions in bus-driver layer can't apply to all Non-PCI MSI devices, because we can not guarantee Non-PCI MSI devices are always on a bus. The existing HPET, DMAR device both have no bus bind. Yes, that's why I was not sure of bus-driver or device-driver model. I'm working on a new MSI setup framework, as you mentioned before, in device-driver model. I abstracted a new virtual device (called struct msi_dev), this msi_dev will manage all MSI info, Will this struct msi_dev will be part of struct device? and a new bus named msi_bus, also introduced a new driver msi_driver, msi_bus is responsible for binding msi_dev and msi_driver. All MSI devices will be classified into different MSI device types, like MSI_TYPE_PCI, MSI_TYPE_HPET, MSI_TYPE_DMAR, etc.. Each MSI type device should provide a private struct msi_driver. msi_driver should contain the type specific MSI ops functions to help setup and enable MSI device, request MSI irq. I almost finish the first draft, and will post out next week in plan :) Will be looking forward to next version. Hi Bharat, I'm sorry I had to delay to send out the new version :(. I found some risks in the new MSI framework, i.e. DMAR MSI initialized the MSI before the linux device-driver tree be built. And we also found some problems during test. So I think I need more time to review and test. Thanks! Yijing. Thanks -Bharat Thanks! Yijing. Thanks -Bharat My patchset is just a RFC draft, I will update it later, all we want to do is make kernel support Non-PCI MSI devices. Thanks! Yijing. Thanks Arnab -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ . -- Thanks! Yijing -- To unsubscribe from this list: send the line unsubscribe linux-pci in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html . -- Thanks! Yijing -- To unsubscribe from this list: send the line unsubscribe linux-pci in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ . -- Thanks! Yijing ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
The key difference between PCI device and Non-PCI MSI is the interfaces to access hardware MSI registers. for instance, currently, msi_chip-setup_irq() to setup MSI irq and configure the MSI address/data registers, so we need to provide device specific write_msi_msg() interface, then when we call msi_chip-setup_irq(), the device MSI registers can be configured appropriately. What if we can register/override the setup_irq() from bus-driver (not sure, but may be device-driver itself). Example PCI bus-driver will provide setup_irq() (or the part of setup_irq which set address and data in h/w) by PCI bus, which configure address/data in h/w as per PCI standard. We in Freescale will be using MSI for the devices behind a new-bus (which is not PCI based), We have a separate bus driver for same. And this new bus driver register/provide its own address/data write function which is based on that specific bus protocol. Hi Bharat, I'm glad to know your MSI device working mode. Provide the private MSI setup functions in bus-driver layer can't apply to all Non-PCI MSI devices, because we can not guarantee Non-PCI MSI devices are always on a bus. The existing HPET, DMAR device both have no bus bind. I'm working on a new MSI setup framework, as you mentioned before, in device-driver model. I abstracted a new virtual device (called struct msi_dev), this msi_dev will manage all MSI info, and a new bus named msi_bus, also introduced a new driver msi_driver, msi_bus is responsible for binding msi_dev and msi_driver. All MSI devices will be classified into different MSI device types, like MSI_TYPE_PCI, MSI_TYPE_HPET, MSI_TYPE_DMAR, etc.. Each MSI type device should provide a private struct msi_driver. msi_driver should contain the type specific MSI ops functions to help setup and enable MSI device, request MSI irq. I almost finish the first draft, and will post out next week in plan :) Thanks! Yijing. Thanks -Bharat My patchset is just a RFC draft, I will update it later, all we want to do is make kernel support Non-PCI MSI devices. Thanks! Yijing. Thanks Arnab -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ . -- Thanks! Yijing -- To unsubscribe from this list: send the line unsubscribe linux-pci in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html . -- Thanks! Yijing ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
RE: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
-Original Message- From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci-ow...@vger.kernel.org] On Behalf Of Yijing Wang Sent: Wednesday, August 20, 2014 11:59 AM To: Bhushan Bharat-R65777; Basu Arnab-B45036 Cc: Xinwei Hu; Wuyun; Bjorn Helgaas; linux-...@vger.kernel.org; paul.mu...@huawei.com; James E.J. Bottomley; Marc Zyngier; linux-arm- ker...@lists.infradead.org; Russell King; linux-a...@vger.kernel.org; virtualization@lists.linux-foundation.org; Hanjun Guo; linux- ker...@vger.kernel.org Subject: Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device The key difference between PCI device and Non-PCI MSI is the interfaces to access hardware MSI registers. for instance, currently, msi_chip-setup_irq() to setup MSI irq and configure the MSI address/data registers, so we need to provide device specific write_msi_msg() interface, then when we call msi_chip-setup_irq(), the device MSI registers can be configured appropriately. What if we can register/override the setup_irq() from bus-driver (not sure, but may be device-driver itself). Example PCI bus-driver will provide setup_irq() (or the part of setup_irq which set address and data in h/w) by PCI bus, which configure address/data in h/w as per PCI standard. We in Freescale will be using MSI for the devices behind a new-bus (which is not PCI based), We have a separate bus driver for same. And this new bus driver register/provide its own address/data write function which is based on that specific bus protocol. Hi Bharat, I'm glad to know your MSI device working mode. Provide the private MSI setup functions in bus-driver layer can't apply to all Non-PCI MSI devices, because we can not guarantee Non-PCI MSI devices are always on a bus. The existing HPET, DMAR device both have no bus bind. Yes, that's why I was not sure of bus-driver or device-driver model. I'm working on a new MSI setup framework, as you mentioned before, in device-driver model. I abstracted a new virtual device (called struct msi_dev), this msi_dev will manage all MSI info, Will this struct msi_dev will be part of struct device? and a new bus named msi_bus, also introduced a new driver msi_driver, msi_bus is responsible for binding msi_dev and msi_driver. All MSI devices will be classified into different MSI device types, like MSI_TYPE_PCI, MSI_TYPE_HPET, MSI_TYPE_DMAR, etc.. Each MSI type device should provide a private struct msi_driver. msi_driver should contain the type specific MSI ops functions to help setup and enable MSI device, request MSI irq. I almost finish the first draft, and will post out next week in plan :) Will be looking forward to next version. Thanks -Bharat Thanks! Yijing. Thanks -Bharat My patchset is just a RFC draft, I will update it later, all we want to do is make kernel support Non-PCI MSI devices. Thanks! Yijing. Thanks Arnab -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ . -- Thanks! Yijing -- To unsubscribe from this list: send the line unsubscribe linux-pci in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html . -- Thanks! Yijing -- To unsubscribe from this list: send the line unsubscribe linux-pci in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
We in Freescale will be using MSI for the devices behind a new-bus (which is
not PCI based), We have a separate bus driver for same. And this new bus
driver
register/provide its own address/data write function which is based on that
specific bus protocol.
Hi Bharat, I'm glad to know your MSI device working mode.
Provide the private MSI setup functions in bus-driver layer can't apply to
all
Non-PCI MSI devices, because we can not guarantee Non-PCI MSI devices are
always
on a bus. The existing HPET, DMAR device both have no bus bind.
Yes, that's why I was not sure of bus-driver or device-driver model.
I'm working on a
new MSI setup framework, as you mentioned before, in device-driver model.
I abstracted a new virtual device (called struct msi_dev), this msi_dev will
manage all MSI info,
Will this struct msi_dev will be part of struct device?
struct msi_dev contains the struct device
piece code:
struct msi_dev {
u8 type;
u8 enabled;
u8 nvec;
u8 nvec_retry;
char *id;
void __iomem *base;
struct msix_entry *entries;
struct list_head msi_list;
struct device dev;
void *msi_data;
struct msi_driver *driver;
const struct attribute_group **irq_groups;
};
struct msi_driver {
const char *name;
char *id;
void (*msi_set_enable)(struct msi_dev *dev, int enable);
int (*msi_setup_entry)(struct msi_dev *dev, struct msi_desc *entry);
int (*msix_setup_entries)(struct msi_dev *dev, struct msi_desc *entry, int
index);
u32 (*msi_mask_irq)(struct msi_desc *desc, u32 mask, u32 flag);
u32 (*msix_mask_irq)(struct msi_desc *desc, u32 flag);
void (*msi_read_message)(struct msi_desc *desc, struct msi_msg *msg);
void (*msi_write_message)(struct msi_desc *desc, struct msi_msg *msg);
void (*msi_set_legacy_irq)(struct msi_dev *dev, int enable);
struct device_driver driver;
};
Thanks!
Yijing.
and a new bus named msi_bus, also introduced a new driver
msi_driver, msi_bus is responsible for binding msi_dev and msi_driver.
All MSI devices will be classified into different MSI device types, like
MSI_TYPE_PCI, MSI_TYPE_HPET, MSI_TYPE_DMAR, etc..
Each MSI type device should provide a private struct msi_driver. msi_driver
should contain the type specific MSI ops functions to help setup and enable
MSI
device, request MSI irq.
I almost finish the first draft, and will post out next week in plan :)
Will be looking forward to next version.
Thanks
-Bharat
Thanks!
Yijing.
Thanks
-Bharat
My patchset is just a RFC draft, I will update it later, all we want
to do is make kernel support Non-PCI MSI devices.
Thanks!
Yijing.
Thanks
Arnab
--
To unsubscribe from this list: send the line unsubscribe
linux-kernel in the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
.
--
Thanks!
Yijing
--
To unsubscribe from this list: send the line unsubscribe linux-pci
in the body of a message to majord...@vger.kernel.org More majordomo
info at http://vger.kernel.org/majordomo-info.html
.
--
Thanks!
Yijing
--
To unsubscribe from this list: send the line unsubscribe linux-pci in the
body
of a message to majord...@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
.
--
Thanks!
Yijing
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
RE: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
Hi Yijing -Original Message- From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci-ow...@vger.kernel.org] On Behalf Of Yijing Wang Sent: Monday, August 04, 2014 8:34 AM To: Basu Arnab-B45036 Cc: Xinwei Hu; Wuyun; Bjorn Helgaas; linux-...@vger.kernel.org; paul.mu...@huawei.com; James E.J. Bottomley; Marc Zyngier; linux-arm- ker...@lists.infradead.org; Russell King; linux-a...@vger.kernel.org; virtualization@lists.linux-foundation.org; Hanjun Guo; linux- ker...@vger.kernel.org Subject: Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device MSI was introduced in PCI Spec 2.2. Currently, kernel MSI driver codes are bonding with PCI device. Because MSI has a lot advantages in design. More and more non-PCI devices want to use MSI as their default interrupt. The existing MSI device include HPET. HPET driver provide its own MSI code to initialize and process MSI interrupts. In the latest GIC v3 spec, legacy device can deliver MSI by the help of a relay device named consolidator. Consolidator can translate the legacy interrupts connected to it to MSI/MSI-X. And new non-PCI device will be designed to support MSI in future. So make the MSI driver code be generic will help the non-PCI device use MSI more simply. As per my understanding the GICv3 provides a service that will convert writes to a specified address to IRQs delivered to the core and as you mention above MSIs are part of the PCI spec. So I can see a strong case for non-PCI devices to want MSI like functionality without being fully compliant with the requirements of the MSI spec. In GICv3, MBI is named for the service, but there is no more detailed information about it, only we can know is MBI is analogous to MSI, MBI devices must have address/data registers, but other registers like enable/mask/ctrl are not mandatory requirement. I don't know whether the MBI spec will be release, but anyway I think MSI refactoring is make sense, there are some existing Non-PCI MSI device like hpet, dmar. For simplicity, let name MSI and MBI to MSI temporarily. My question is do we necessarily want to rework so much of the PCI-MSI layer to support non PCI devices? Or will it be sufficient to create a framework to allow non PCI devices to hook up with a device that can convert their writes to an IRQ to the core. As I understand it, the msi_chip is (almost) such a framework. The only problem being that it makes some PCI specific assumptions (such as PCI specific writes from within msi_chip functions). Won't it be sufficient to make the msi_chip framework generic enough to be used by non-PCI devices and let each bus/device manage any additional requirements (such as configuration flow, bit definitions etc) that it places on message based interrupts? msi_chip framework is important to support that, but I think maybe it's not enough, msi_chip is only responsible for IRQ allocation, teardown, etc.. The key difference between PCI device and Non-PCI MSI is the interfaces to access hardware MSI registers. for instance, currently, msi_chip-setup_irq() to setup MSI irq and configure the MSI address/data registers, so we need to provide device specific write_msi_msg() interface, then when we call msi_chip-setup_irq(), the device MSI registers can be configured appropriately. What if we can register/override the setup_irq() from bus-driver (not sure, but may be device-driver itself). Example PCI bus-driver will provide setup_irq() (or the part of setup_irq which set address and data in h/w) by PCI bus, which configure address/data in h/w as per PCI standard. We in Freescale will be using MSI for the devices behind a new-bus (which is not PCI based), We have a separate bus driver for same. And this new bus driver register/provide its own address/data write function which is based on that specific bus protocol. Thanks -Bharat My patchset is just a RFC draft, I will update it later, all we want to do is make kernel support Non-PCI MSI devices. Thanks! Yijing. Thanks Arnab -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ . -- Thanks! Yijing -- To unsubscribe from this list: send the line unsubscribe linux-pci in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
On 2014/8/1 21:52, Arnd Bergmann wrote:
On Wednesday 30 July 2014, Yijing Wang wrote:
On 2014/7/29 22:08, Arnd Bergmann wrote:
On Saturday 26 July 2014 11:08:37 Yijing Wang wrote:
The new data struct for generic MSI driver.
struct msi_irqs {
u8 msi_enabled:1; /* Enable flag */
u8 msix_enabled:1;
struct list_head msi_list; /* MSI desc list */
void *data; /* help to find the MSI device */
struct msi_ops *ops; /* MSI device specific hook */
};
struct msi_irqs is used to manage MSI related informations. Every device
supports
MSI should contain this data struct and allocate it.
I think you should have a stronger association with the 'struct
device' here. Can you replace the 'void *data' with 'struct device *dev'?
Actually, I used the struct device *dev in my first draft, finally, I
replaced
it with void *data, because some MSI devices don't have a struct device *dev,
like the existing hpet device, dmar msi device, and OF device, like the ARM
consolidator.
Of course, we can make the MSI devices have their own struct device, and
register to
device tree, eg, add a class device named MSI_DEV. But I'm not sure whether
it is appropriate.
It doesn't have to be in the (OF) device tree, but I think it absolutely makes
sense to use the 'struct device' infrastructure here, as almost everything
uses
a device, and the ones that don't do that today can be easily changed.
I will try to use struct device infrastructure, thanks for your suggestion. :)
The other part I'm not completely sure about is how you want to
have MSIs map into normal IRQ descriptors. At the moment, all
MSI users are based on IRQ numbers, but this has known scalability problems.
Hmmm, I still use the IRQ number to map the MSIs to IRQ description.
I'm sorry, I don't understand you meaning.
What are the scalability problems you mentioned ?
For device drivers, they always process interrupt in two steps.
If irq is the legacy interrupt, drivers will first
use the irq_of_parse_and_map() or pci_enable_device() to parse and get the
IRQ number.
Then drivers will call the request_irq() to register the interrupt handler.
If irq is MSIs, first call pci_enable_msi/x() to get the IRQ number and then
call
request_irq() to register interrupt handler.
The method you describe here makes sense for PCI devices that are required to
support
legacy interrupts and may or may not support MSI on a given system, but not
so much
for platform devices for which we know exactly whether we want to use MSI
or legacy interrupts.
In particular if you have a device that can only do MSI, the entire
pci_enable_msi
step is pointless: all we need to do is program the correct MSI target
address/message
pair into the device and register the handler.
Yes, I almost agree if we won't change the existing hundreds of drivers, what
I worried about is some drivers may want to know the IRQ numbers, and use the
IRQ
number to process different things, as I mentioned in another reply.
But we can also provide the interface which integrate MSI configuration and
request_irq(),
if most drivers don't care the IRQ number.
I wonder if we can do the interface in a way that
hides the interrupt number from generic device drivers and just
passes a 'struct irq_desc'. Note that there are long-term plans to
get rid of IRQ numbers entirely, but those plans have existed for
a long time already without anybody seriously addressing the device
driver interfaces so far, so it might never really happen.
Maybe this is a huge work, now hundreds drivers use the IRQ number, so maybe
we can consider
this in a separate title.
Sorry for being unclear here: I did suggest changing all drivers now. What I
meant
is that we use a different API for non-PCI devices that works without IRQ
numbers.
I don't think we should touch the PCI interfaces at this point.
OK, I got it.
What I'd envision as the API from the device driver perspective is something
as simple like this:
struct msi_desc *msi_request(struct msi_chip *chip, irq_handler_t handler,
unsigned long flags, const char *name, struct device
*dev);
which would get an msi descriptor that is valid for this device (dev)
connected to a particular msi_chip, and associate a handler function
with it. The device driver can call that function and retrieve the
address/message pair from the msi_desc in order to store it in its own
device specific registers. The request_irq() can be handled internally
to msi_request().
This is a huge change for device drivers, and some device drivers don't know
which msi_chip
their MSI irq deliver to. I'm reworking the msi_chip, and try to use
msi_chip to eliminate
all arch_msi_xxx() under every arch in kernel. And the important point is
how to create the
binding for the MSI device to the target msi_chip.
Which drivers are you thinking of? Again, I wouldn't expect to
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
On Monday 04 August 2014, Yijing Wang wrote: I have another question is some drivers will request more than one MSI/MSI-X IRQ, and the driver will use them to process different things. Eg. network driver generally uses one of them to process trivial network thins, and others to transmit/receive data. So, in this case, it seems to driver need to touch the IRQ numbers. wr-linux:~ # cat /proc/interrupts CPU0 CPU1 CPU2 CPU17 CPU18 CPU19 CPU20 CPU21 CPU22 CPU23 .. 100: 0 0 0 0 0 0 0 0 0 0 IR-PCI-MSI-edge eth0 101: 2 0 0 0 0 0 302830488 0 0 0 IR-PCI-MSI-edge eth0-TxRx-0 102:110 0 0 0 0 360675897 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-1 103:109 0 0 0 0 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-2 104:107 0 0 9678933 0 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-3 105:107 0 0 0 357838258 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-4 106:115 0 0 0 0 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-5 107:114 0 0 0 0 0 0 337866096 0 0 IR-PCI-MSI-edge eth0-TxRx-6 108: 373801199 0 0 0 0 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-7 I think in this example, you just need to request eight interrupts, and pass a different data pointer each time, pointing to the napi_struct of each of the NIC queues. The driver has no need to deal with the IRQ number at all, and I would be surprised if it cared today. Arnd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
On Monday 04 August 2014, Yijing Wang wrote: On 2014/8/1 21:52, Arnd Bergmann wrote: On Wednesday 30 July 2014, Yijing Wang wrote: On 2014/7/29 22:08, Arnd Bergmann wrote: The other part I'm not completely sure about is how you want to have MSIs map into normal IRQ descriptors. At the moment, all MSI users are based on IRQ numbers, but this has known scalability problems. Hmmm, I still use the IRQ number to map the MSIs to IRQ description. I'm sorry, I don't understand you meaning. What are the scalability problems you mentioned ? For device drivers, they always process interrupt in two steps. If irq is the legacy interrupt, drivers will first use the irq_of_parse_and_map() or pci_enable_device() to parse and get the IRQ number. Then drivers will call the request_irq() to register the interrupt handler. If irq is MSIs, first call pci_enable_msi/x() to get the IRQ number and then call request_irq() to register interrupt handler. The method you describe here makes sense for PCI devices that are required to support legacy interrupts and may or may not support MSI on a given system, but not so much for platform devices for which we know exactly whether we want to use MSI or legacy interrupts. In particular if you have a device that can only do MSI, the entire pci_enable_msi step is pointless: all we need to do is program the correct MSI target address/message pair into the device and register the handler. Yes, I almost agree if we won't change the existing hundreds of drivers, what I worried about is some drivers may want to know the IRQ numbers, and use the IRQ number to process different things, as I mentioned in another reply. But we can also provide the interface which integrate MSI configuration and request_irq(), if most drivers don't care the IRQ number. The driver would still have the option of getting the IRQ number for now: With the interface I imagine, you would get a 'struct msi_desc' pointer, from which you can look up the 'struct irq_desc' pointer (either embedded in msi_desc, or using a pointer from a member of msi_desc), and you can already get the interrupt number from the irq_desc. My point was that a well-written driver already does not care about the interrupt number: the only information a driver needs in the interrupt handler is a pointer to its own context, which we already derive from the irq_desc. The main interface that currently requires the irq number is free_irq(), but I would argue that we can just add a wrapper that takes the msi_desc pointer as its first argument so the driver does not have to worry about it. We can add additional wrappers like that as needed. What I'd envision as the API from the device driver perspective is something as simple like this: struct msi_desc *msi_request(struct msi_chip *chip, irq_handler_t handler, unsigned long flags, const char *name, struct device *dev); which would get an msi descriptor that is valid for this device (dev) connected to a particular msi_chip, and associate a handler function with it. The device driver can call that function and retrieve the address/message pair from the msi_desc in order to store it in its own device specific registers. The request_irq() can be handled internally to msi_request(). This is a huge change for device drivers, and some device drivers don't know which msi_chip their MSI irq deliver to. I'm reworking the msi_chip, and try to use msi_chip to eliminate all arch_msi_xxx() under every arch in kernel. And the important point is how to create the binding for the MSI device to the target msi_chip. Which drivers are you thinking of? Again, I wouldn't expect to change any PCI drivers, but only platform drivers that do native MSI, so we only have to change drivers that do not support any MSI at all yet and that need to be changed anyway in order to add support. I mean platform device drivers, because we can find the target msi_chip by some platform interfaces(like the existing of_pci_find_msi_chip_by_node()). So we no need to explicitly provide the msi_chip as the function argument. Right, that works too. I was thinking we might need an interface that allows us to pick a particular msi_chip if there are several alternatives (e.g. one in the GIC and one in the PCI host), but you are right: we should normally be able to hardwire that information in DT or elsewhere, and just need the 'struct device pointer' which should probably be the first argument here. As you pointed out, it's common to have multiple MSIs for a single device, so we also need a context to pass around, so my suggestion would become something like: struct msi_desc *msi_request(struct device *dev, irq_handler_t handler, unsigned long flags, const char *name, void *data); It's possible that we have to add one or two more arguments
RE: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
Hi Yijing -Original Message- From: Yijing Wang [mailto:wangyij...@huawei.com] Sent: Saturday, July 26, 2014 8:39 AM To: linux-ker...@vger.kernel.org Cc: Xinwei Hu; Wuyun; Bjorn Helgaas; linux-...@vger.kernel.org; paul.mu...@huawei.com; James E.J. Bottomley; Marc Zyngier; linux-arm- ker...@lists.infradead.org; Russell King; linux-a...@vger.kernel.org; Basu Arnab-B45036; virtualization@lists.linux-foundation.org; Hanjun Guo; Yijing Wang Subject: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device Hi all, The series is a draft of generic MSI driver that supports PCI and Non-PCI device which have MSI capability. If you're not interested it, sorry for the noise. Thanks for sending out these patches, I have some (very basic) questions. The series is based on Linux-3.16-rc1. MSI was introduced in PCI Spec 2.2. Currently, kernel MSI driver codes are bonding with PCI device. Because MSI has a lot advantages in design. More and more non-PCI devices want to use MSI as their default interrupt. The existing MSI device include HPET. HPET driver provide its own MSI code to initialize and process MSI interrupts. In the latest GIC v3 spec, legacy device can deliver MSI by the help of a relay device named consolidator. Consolidator can translate the legacy interrupts connected to it to MSI/MSI-X. And new non-PCI device will be designed to support MSI in future. So make the MSI driver code be generic will help the non-PCI device use MSI more simply. As per my understanding the GICv3 provides a service that will convert writes to a specified address to IRQs delivered to the core and as you mention above MSIs are part of the PCI spec. So I can see a strong case for non-PCI devices to want MSI like functionality without being fully compliant with the requirements of the MSI spec. My question is do we necessarily want to rework so much of the PCI-MSI layer to support non PCI devices? Or will it be sufficient to create a framework to allow non PCI devices to hook up with a device that can convert their writes to an IRQ to the core. As I understand it, the msi_chip is (almost) such a framework. The only problem being that it makes some PCI specific assumptions (such as PCI specific writes from within msi_chip functions). Won't it be sufficient to make the msi_chip framework generic enough to be used by non-PCI devices and let each bus/device manage any additional requirements (such as configuration flow, bit definitions etc) that it places on message based interrupts? Thanks Arnab ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
The method you describe here makes sense for PCI devices that are required to support legacy interrupts and may or may not support MSI on a given system, but not so much for platform devices for which we know exactly whether we want to use MSI or legacy interrupts. In particular if you have a device that can only do MSI, the entire pci_enable_msi step is pointless: all we need to do is program the correct MSI target address/message pair into the device and register the handler. Yes, I almost agree if we won't change the existing hundreds of drivers, what I worried about is some drivers may want to know the IRQ numbers, and use the IRQ number to process different things, as I mentioned in another reply. But we can also provide the interface which integrate MSI configuration and request_irq(), if most drivers don't care the IRQ number. The driver would still have the option of getting the IRQ number for now: With the interface I imagine, you would get a 'struct msi_desc' pointer, from which you can look up the 'struct irq_desc' pointer (either embedded in msi_desc, or using a pointer from a member of msi_desc), and you can already get the interrupt number from the irq_desc. My point was that a well-written driver already does not care about the interrupt number: the only information a driver needs in the interrupt handler is a pointer to its own context, which we already derive from the irq_desc. Agree, I will try to introduce this similar interface in next version, thanks! The main interface that currently requires the irq number is free_irq(), but I would argue that we can just add a wrapper that takes the msi_desc pointer as its first argument so the driver does not have to worry about it. We can add additional wrappers like that as needed. OK This is a huge change for device drivers, and some device drivers don't know which msi_chip their MSI irq deliver to. I'm reworking the msi_chip, and try to use msi_chip to eliminate all arch_msi_xxx() under every arch in kernel. And the important point is how to create the binding for the MSI device to the target msi_chip. Which drivers are you thinking of? Again, I wouldn't expect to change any PCI drivers, but only platform drivers that do native MSI, so we only have to change drivers that do not support any MSI at all yet and that need to be changed anyway in order to add support. I mean platform device drivers, because we can find the target msi_chip by some platform interfaces(like the existing of_pci_find_msi_chip_by_node()). So we no need to explicitly provide the msi_chip as the function argument. Right, that works too. I was thinking we might need an interface that allows us to pick a particular msi_chip if there are several alternatives (e.g. one in the GIC and one in the PCI host), but you are right: we should normally be able to hardwire that information in DT or elsewhere, and just need the 'struct device pointer' which should probably be the first argument here. As you pointed out, it's common to have multiple MSIs for a single device, so we also need a context to pass around, so my suggestion would become something like: struct msi_desc *msi_request(struct device *dev, irq_handler_t handler, unsigned long flags, const char *name, void *data); It's possible that we have to add one or two more arguments here. Good suggestion, thanks! A degenerate case of this would be a system where a PCI device sends its MSI into the host controller, that generates a legacy interrupt and that in turn gets sent to an irqchip which turns it back into an MSI for the GICv3. This would of course be very inefficient, but I think we should be able to express this with both the binding and the in-kernel framework just to be on the safe side. Yes, the best way to tell the kernel which msi_chip should deliver to is describe the binding in DTS file. If a real degenerate case found, we can update the platform interface which is responsible for getting the match msi_chip in future. Ok. Arnd . -- Thanks! Yijing ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
On 2014/8/4 22:45, Arnd Bergmann wrote: On Monday 04 August 2014, Yijing Wang wrote: I have another question is some drivers will request more than one MSI/MSI-X IRQ, and the driver will use them to process different things. Eg. network driver generally uses one of them to process trivial network thins, and others to transmit/receive data. So, in this case, it seems to driver need to touch the IRQ numbers. wr-linux:~ # cat /proc/interrupts CPU0 CPU1 CPU2 CPU17 CPU18 CPU19 CPU20 CPU21 CPU22 CPU23 .. 100: 0 0 0 0 0 0 0 0 0 0 IR-PCI-MSI-edge eth0 101: 2 0 0 0 0 0 302830488 0 0 0 IR-PCI-MSI-edge eth0-TxRx-0 102:110 0 0 0 0 360675897 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-1 103:109 0 0 0 0 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-2 104:107 0 0 9678933 0 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-3 105:107 0 0 0 357838258 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-4 106:115 0 0 0 0 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-5 107:114 0 0 0 0 0 0 337866096 0 0 IR-PCI-MSI-edge eth0-TxRx-6 108: 373801199 0 0 0 0 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-7 I think in this example, you just need to request eight interrupts, and pass a different data pointer each time, pointing to the napi_struct of each of the NIC queues. The driver has no need to deal with the IRQ number at all, and I would be surprised if it cared today. Yes, you are right, this is not a stumbling block. :) Arnd . -- Thanks! Yijing ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
MSI was introduced in PCI Spec 2.2. Currently, kernel MSI driver codes are bonding with PCI device. Because MSI has a lot advantages in design. More and more non-PCI devices want to use MSI as their default interrupt. The existing MSI device include HPET. HPET driver provide its own MSI code to initialize and process MSI interrupts. In the latest GIC v3 spec, legacy device can deliver MSI by the help of a relay device named consolidator. Consolidator can translate the legacy interrupts connected to it to MSI/MSI-X. And new non-PCI device will be designed to support MSI in future. So make the MSI driver code be generic will help the non-PCI device use MSI more simply. As per my understanding the GICv3 provides a service that will convert writes to a specified address to IRQs delivered to the core and as you mention above MSIs are part of the PCI spec. So I can see a strong case for non-PCI devices to want MSI like functionality without being fully compliant with the requirements of the MSI spec. In GICv3, MBI is named for the service, but there is no more detailed information about it, only we can know is MBI is analogous to MSI, MBI devices must have address/data registers, but other registers like enable/mask/ctrl are not mandatory requirement. I don't know whether the MBI spec will be release, but anyway I think MSI refactoring is make sense, there are some existing Non-PCI MSI device like hpet, dmar. For simplicity, let name MSI and MBI to MSI temporarily. My question is do we necessarily want to rework so much of the PCI-MSI layer to support non PCI devices? Or will it be sufficient to create a framework to allow non PCI devices to hook up with a device that can convert their writes to an IRQ to the core. As I understand it, the msi_chip is (almost) such a framework. The only problem being that it makes some PCI specific assumptions (such as PCI specific writes from within msi_chip functions). Won't it be sufficient to make the msi_chip framework generic enough to be used by non-PCI devices and let each bus/device manage any additional requirements (such as configuration flow, bit definitions etc) that it places on message based interrupts? msi_chip framework is important to support that, but I think maybe it's not enough, msi_chip is only responsible for IRQ allocation, teardown, etc.. The key difference between PCI device and Non-PCI MSI is the interfaces to access hardware MSI registers. for instance, currently, msi_chip-setup_irq() to setup MSI irq and configure the MSI address/data registers, so we need to provide device specific write_msi_msg() interface, then when we call msi_chip-setup_irq(), the device MSI registers can be configured appropriately. My patchset is just a RFC draft, I will update it later, all we want to do is make kernel support Non-PCI MSI devices. Thanks! Yijing. Thanks Arnab -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ . -- Thanks! Yijing ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
On 2014/8/1 21:16, Arnd Bergmann wrote: On Wednesday 30 July 2014, Yijing Wang wrote: The other part I'm not completely sure about is how you want to have MSIs map into normal IRQ descriptors. At the moment, all MSI users are based on IRQ numbers, but this has known scalability problems. Hmmm, I still use the IRQ number to map the MSIs to IRQ description. I'm sorry, I don't understand you meaning. What are the scalability problems you mentioned ? We have soft limitation of nr_irqs or hard limitation NR_IRQS, we couldn't allocate as much irq number as we need in some cases, such as to support MSI-x. Oh, yes, this is a potential issue. Gerry, thanks for you explanation. :) This should no longer be an issue, as arm64 uses CONFIG_SPARSE_IRQ and the number of interrupts is not limited in any form. My point was more that the device driver should not need to care about the interrupt number: it gets made up on the spot when the MSI is needed, and then it is only used to request the IRQ. This can be simplified into one interface at the device driver level, even though the internal still use numbers somewhere. If we ever remove IRQ numbers from the driver API, this part doesn't need to get touched again. Hi Arnd, I have another question is some drivers will request more than one MSI/MSI-X IRQ, and the driver will use them to process different things. Eg. network driver generally uses one of them to process trivial network thins, and others to transmit/receive data. So, in this case, it seems to driver need to touch the IRQ numbers. wr-linux:~ # cat /proc/interrupts CPU0 CPU1 CPU2 CPU17 CPU18 CPU19 CPU20 CPU21 CPU22 CPU23 .. 100: 0 0 0 0 0 0 0 0 0 0 IR-PCI-MSI-edge eth0 101: 2 0 0 0 0 0 302830488 0 0 0 IR-PCI-MSI-edge eth0-TxRx-0 102:110 0 0 0 0 360675897 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-1 103:109 0 0 0 0 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-2 104:107 0 0 9678933 0 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-3 105:107 0 0 0 357838258 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-4 106:115 0 0 0 0 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-5 107:114 0 0 0 0 0 0 337866096 0 0 IR-PCI-MSI-edge eth0-TxRx-6 108: 373801199 0 0 0 0 0 0 0 0 0 IR-PCI-MSI-edge eth0-TxRx-7 Thanks! Yijing. . -- Thanks! Yijing ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
On Wednesday 30 July 2014, Yijing Wang wrote: The other part I'm not completely sure about is how you want to have MSIs map into normal IRQ descriptors. At the moment, all MSI users are based on IRQ numbers, but this has known scalability problems. Hmmm, I still use the IRQ number to map the MSIs to IRQ description. I'm sorry, I don't understand you meaning. What are the scalability problems you mentioned ? We have soft limitation of nr_irqs or hard limitation NR_IRQS, we couldn't allocate as much irq number as we need in some cases, such as to support MSI-x. Oh, yes, this is a potential issue. Gerry, thanks for you explanation. :) This should no longer be an issue, as arm64 uses CONFIG_SPARSE_IRQ and the number of interrupts is not limited in any form. My point was more that the device driver should not need to care about the interrupt number: it gets made up on the spot when the MSI is needed, and then it is only used to request the IRQ. This can be simplified into one interface at the device driver level, even though the internal still use numbers somewhere. If we ever remove IRQ numbers from the driver API, this part doesn't need to get touched again. Arnd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
On Wednesday 30 July 2014, Yijing Wang wrote:
On 2014/7/29 22:08, Arnd Bergmann wrote:
On Saturday 26 July 2014 11:08:37 Yijing Wang wrote:
The new data struct for generic MSI driver.
struct msi_irqs {
u8 msi_enabled:1; /* Enable flag */
u8 msix_enabled:1;
struct list_head msi_list; /* MSI desc list */
void *data; /* help to find the MSI device */
struct msi_ops *ops; /* MSI device specific hook */
};
struct msi_irqs is used to manage MSI related informations. Every device
supports
MSI should contain this data struct and allocate it.
I think you should have a stronger association with the 'struct
device' here. Can you replace the 'void *data' with 'struct device *dev'?
Actually, I used the struct device *dev in my first draft, finally, I replaced
it with void *data, because some MSI devices don't have a struct device *dev,
like the existing hpet device, dmar msi device, and OF device, like the ARM
consolidator.
Of course, we can make the MSI devices have their own struct device, and
register to
device tree, eg, add a class device named MSI_DEV. But I'm not sure whether
it is appropriate.
It doesn't have to be in the (OF) device tree, but I think it absolutely makes
sense to use the 'struct device' infrastructure here, as almost everything uses
a device, and the ones that don't do that today can be easily changed.
The other part I'm not completely sure about is how you want to
have MSIs map into normal IRQ descriptors. At the moment, all
MSI users are based on IRQ numbers, but this has known scalability problems.
Hmmm, I still use the IRQ number to map the MSIs to IRQ description.
I'm sorry, I don't understand you meaning.
What are the scalability problems you mentioned ?
For device drivers, they always process interrupt in two steps.
If irq is the legacy interrupt, drivers will first
use the irq_of_parse_and_map() or pci_enable_device() to parse and get the
IRQ number.
Then drivers will call the request_irq() to register the interrupt handler.
If irq is MSIs, first call pci_enable_msi/x() to get the IRQ number and then
call
request_irq() to register interrupt handler.
The method you describe here makes sense for PCI devices that are required to
support
legacy interrupts and may or may not support MSI on a given system, but not so
much
for platform devices for which we know exactly whether we want to use MSI
or legacy interrupts.
In particular if you have a device that can only do MSI, the entire
pci_enable_msi
step is pointless: all we need to do is program the correct MSI target
address/message
pair into the device and register the handler.
I wonder if we can do the interface in a way that
hides the interrupt number from generic device drivers and just
passes a 'struct irq_desc'. Note that there are long-term plans to
get rid of IRQ numbers entirely, but those plans have existed for
a long time already without anybody seriously addressing the device
driver interfaces so far, so it might never really happen.
Maybe this is a huge work, now hundreds drivers use the IRQ number, so maybe
we can consider
this in a separate title.
Sorry for being unclear here: I did suggest changing all drivers now. What I
meant
is that we use a different API for non-PCI devices that works without IRQ
numbers.
I don't think we should touch the PCI interfaces at this point.
With the other operations, I think they should all take a 'struct device *'
as the first argument for convenience and consistency. I don't think you
actually
need msi_read_message(), and we could avoid msi_write_message() by doing it
the other way round.
There only two functions use the read_msi_msg(), because every msi_desc has
a struct msi_msg, and it caches the msi address and data. I will consider to
retrieve the msg from cached msi_msg, then we can avoid the
msi_read_message().
But msi_write_message() maybe necessary, some xxx_set_affinity() functions and
restore functions need the msi_write_message() to rewrite the address and
data.
Makes sense. I'd have to think about it more, but I think you are right
about the affinity APIs needing this.
What I'd envision as the API from the device driver perspective is something
as simple like this:
struct msi_desc *msi_request(struct msi_chip *chip, irq_handler_t handler,
unsigned long flags, const char *name, struct device
*dev);
which would get an msi descriptor that is valid for this device (dev)
connected to a particular msi_chip, and associate a handler function
with it. The device driver can call that function and retrieve the
address/message pair from the msi_desc in order to store it in its own
device specific registers. The request_irq() can be handled internally
to msi_request().
This is a huge change for device drivers, and some device drivers don't know
which msi_chip
their
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
On 2014/7/30 14:47, Jiang Liu wrote:
On 2014/7/30 10:45, Yijing Wang wrote:
On 2014/7/29 22:08, Arnd Bergmann wrote:
On Saturday 26 July 2014 11:08:37 Yijing Wang wrote:
The series is a draft of generic MSI driver that supports PCI
and Non-PCI device which have MSI capability. If you're not interested
it, sorry for the noise.
I've finally managed to take some time to look at the series. Overall,
the concept looks good to me, and the patches look very well implemented.
The part I'm not sure about is the interface we want to end up with
at the end of the series. More on that below
Hi Arnd,
Thanks for your review and comments very much!
Please refer the inline comments.
The series is based on Linux-3.16-rc1.
MSI was introduced in PCI Spec 2.2. Currently, kernel MSI
driver codes are bonding with PCI device. Because MSI has a lot
advantages in design. More and more non-PCI devices want to
use MSI as their default interrupt. The existing MSI device
include HPET. HPET driver provide its own MSI code to initialize
and process MSI interrupts. In the latest GIC v3 spec, legacy device
can deliver MSI by the help of a relay device named consolidator.
Consolidator can translate the legacy interrupts connected to it
to MSI/MSI-X. And new non-PCI device will be designed to
support MSI in future. So make the MSI driver code be generic will
help the non-PCI device use MSI more simply.
The new data struct for generic MSI driver.
struct msi_irqs {
u8 msi_enabled:1; /* Enable flag */
u8 msix_enabled:1;
struct list_head msi_list; /* MSI desc list */
void *data; /* help to find the MSI device */
struct msi_ops *ops; /* MSI device specific hook */
};
struct msi_irqs is used to manage MSI related informations. Every device
supports
MSI should contain this data struct and allocate it.
I think you should have a stronger association with the 'struct
device' here. Can you replace the 'void *data' with 'struct device *dev'?
Actually, I used the struct device *dev in my first draft, finally, I
replaced
it with void *data, because some MSI devices don't have a struct device *dev,
like the existing hpet device, dmar msi device, and OF device, like the ARM
consolidator.
Of course, we can make the MSI devices have their own struct device, and
register to
device tree, eg, add a class device named MSI_DEV. But I'm not sure whether
it is appropriate.
The other part I'm not completely sure about is how you want to
have MSIs map into normal IRQ descriptors. At the moment, all
MSI users are based on IRQ numbers, but this has known scalability problems.
Hmmm, I still use the IRQ number to map the MSIs to IRQ description.
I'm sorry, I don't understand you meaning.
What are the scalability problems you mentioned ?
We have soft limitation of nr_irqs or hard limitation NR_IRQS,
we couldn't allocate as much irq number as we need in some cases,
such as to support MSI-x.
Oh, yes, this is a potential issue. Gerry, thanks for you explanation. :)
For device drivers, they always process interrupt in two steps.
If irq is the legacy interrupt, drivers will first
use the irq_of_parse_and_map() or pci_enable_device() to parse and get the
IRQ number.
Then drivers will call the request_irq() to register the interrupt handler.
If irq is MSIs, first call pci_enable_msi/x() to get the IRQ number and then
call
request_irq() to register interrupt handler.
I wonder if we can do the interface in a way that
hides the interrupt number from generic device drivers and just
passes a 'struct irq_desc'. Note that there are long-term plans to
get rid of IRQ numbers entirely, but those plans have existed for
a long time already without anybody seriously addressing the device
driver interfaces so far, so it might never really happen.
Maybe this is a huge work, now hundreds drivers use the IRQ number, so maybe
we can consider
this in a separate title.
struct msi_ops {
struct msi_desc *(*msi_setup_entry)(struct msi_irqs *msi, struct
msi_desc *entry);
int msix_setup_entries(struct msi_irqs *msi, struct msix_entry
*entries);
u32 (*msi_mask_irq)(struct msi_desc *desc, u32 mask, u32 flag);
u32 (*msix_mask_irq)(struct msi_desc *desc, u32 flag);
void (*msi_read_message)(struct msi_desc *desc, struct msi_msg
*msg);
void (*msi_write_message)(struct msi_desc *desc, struct msi_msg
*msg);
void (*msi_set_intx)(struct msi_irqs *msi, int enable);
};
struct msi_ops provides several hook functions, generic MSI driver will
call
the hook functions to access device specific registers. PCI devices will
share
the same msi_ops, because they have the same way to access MSI hardware
registers.
Generic MSI layer export msi_capability_init() and msix_capability_init()
functions
to drivers. msi/x_capability_init() will initialize MSI capability data
struct
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
On 2014/7/30 10:45, Yijing Wang wrote:
On 2014/7/29 22:08, Arnd Bergmann wrote:
On Saturday 26 July 2014 11:08:37 Yijing Wang wrote:
The series is a draft of generic MSI driver that supports PCI
and Non-PCI device which have MSI capability. If you're not interested
it, sorry for the noise.
I've finally managed to take some time to look at the series. Overall,
the concept looks good to me, and the patches look very well implemented.
The part I'm not sure about is the interface we want to end up with
at the end of the series. More on that below
Hi Arnd,
Thanks for your review and comments very much!
Please refer the inline comments.
The series is based on Linux-3.16-rc1.
MSI was introduced in PCI Spec 2.2. Currently, kernel MSI
driver codes are bonding with PCI device. Because MSI has a lot
advantages in design. More and more non-PCI devices want to
use MSI as their default interrupt. The existing MSI device
include HPET. HPET driver provide its own MSI code to initialize
and process MSI interrupts. In the latest GIC v3 spec, legacy device
can deliver MSI by the help of a relay device named consolidator.
Consolidator can translate the legacy interrupts connected to it
to MSI/MSI-X. And new non-PCI device will be designed to
support MSI in future. So make the MSI driver code be generic will
help the non-PCI device use MSI more simply.
The new data struct for generic MSI driver.
struct msi_irqs {
u8 msi_enabled:1; /* Enable flag */
u8 msix_enabled:1;
struct list_head msi_list; /* MSI desc list */
void *data; /* help to find the MSI device */
struct msi_ops *ops; /* MSI device specific hook */
};
struct msi_irqs is used to manage MSI related informations. Every device
supports
MSI should contain this data struct and allocate it.
I think you should have a stronger association with the 'struct
device' here. Can you replace the 'void *data' with 'struct device *dev'?
Actually, I used the struct device *dev in my first draft, finally, I replaced
it with void *data, because some MSI devices don't have a struct device *dev,
like the existing hpet device, dmar msi device, and OF device, like the ARM
consolidator.
Of course, we can make the MSI devices have their own struct device, and
register to
device tree, eg, add a class device named MSI_DEV. But I'm not sure whether
it is appropriate.
The other part I'm not completely sure about is how you want to
have MSIs map into normal IRQ descriptors. At the moment, all
MSI users are based on IRQ numbers, but this has known scalability problems.
Hmmm, I still use the IRQ number to map the MSIs to IRQ description.
I'm sorry, I don't understand you meaning.
What are the scalability problems you mentioned ?
We have soft limitation of nr_irqs or hard limitation NR_IRQS,
we couldn't allocate as much irq number as we need in some cases,
such as to support MSI-x.
For device drivers, they always process interrupt in two steps.
If irq is the legacy interrupt, drivers will first
use the irq_of_parse_and_map() or pci_enable_device() to parse and get the
IRQ number.
Then drivers will call the request_irq() to register the interrupt handler.
If irq is MSIs, first call pci_enable_msi/x() to get the IRQ number and then
call
request_irq() to register interrupt handler.
I wonder if we can do the interface in a way that
hides the interrupt number from generic device drivers and just
passes a 'struct irq_desc'. Note that there are long-term plans to
get rid of IRQ numbers entirely, but those plans have existed for
a long time already without anybody seriously addressing the device
driver interfaces so far, so it might never really happen.
Maybe this is a huge work, now hundreds drivers use the IRQ number, so maybe
we can consider
this in a separate title.
struct msi_ops {
struct msi_desc *(*msi_setup_entry)(struct msi_irqs *msi, struct
msi_desc *entry);
int msix_setup_entries(struct msi_irqs *msi, struct msix_entry
*entries);
u32 (*msi_mask_irq)(struct msi_desc *desc, u32 mask, u32 flag);
u32 (*msix_mask_irq)(struct msi_desc *desc, u32 flag);
void (*msi_read_message)(struct msi_desc *desc, struct msi_msg
*msg);
void (*msi_write_message)(struct msi_desc *desc, struct msi_msg
*msg);
void (*msi_set_intx)(struct msi_irqs *msi, int enable);
};
struct msi_ops provides several hook functions, generic MSI driver will call
the hook functions to access device specific registers. PCI devices will
share
the same msi_ops, because they have the same way to access MSI hardware
registers.
Generic MSI layer export msi_capability_init() and msix_capability_init()
functions
to drivers. msi/x_capability_init() will initialize MSI capability data
struct msi_desc
and alloc the irq, then write the msi address/data value to hardware
registers.
This series only did
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
On Saturday 26 July 2014 11:08:37 Yijing Wang wrote:
The series is a draft of generic MSI driver that supports PCI
and Non-PCI device which have MSI capability. If you're not interested
it, sorry for the noise.
I've finally managed to take some time to look at the series. Overall,
the concept looks good to me, and the patches look very well implemented.
The part I'm not sure about is the interface we want to end up with
at the end of the series. More on that below
The series is based on Linux-3.16-rc1.
MSI was introduced in PCI Spec 2.2. Currently, kernel MSI
driver codes are bonding with PCI device. Because MSI has a lot
advantages in design. More and more non-PCI devices want to
use MSI as their default interrupt. The existing MSI device
include HPET. HPET driver provide its own MSI code to initialize
and process MSI interrupts. In the latest GIC v3 spec, legacy device
can deliver MSI by the help of a relay device named consolidator.
Consolidator can translate the legacy interrupts connected to it
to MSI/MSI-X. And new non-PCI device will be designed to
support MSI in future. So make the MSI driver code be generic will
help the non-PCI device use MSI more simply.
The new data struct for generic MSI driver.
struct msi_irqs {
u8 msi_enabled:1; /* Enable flag */
u8 msix_enabled:1;
struct list_head msi_list; /* MSI desc list */
void *data; /* help to find the MSI device */
struct msi_ops *ops; /* MSI device specific hook */
};
struct msi_irqs is used to manage MSI related informations. Every device
supports
MSI should contain this data struct and allocate it.
I think you should have a stronger association with the 'struct
device' here. Can you replace the 'void *data' with 'struct device *dev'?
The other part I'm not completely sure about is how you want to
have MSIs map into normal IRQ descriptors. At the moment, all
MSI users are based on IRQ numbers, but this has known scalability
problems. I wonder if we can do the interface in a way that
hides the interrupt number from generic device drivers and just
passes a 'struct irq_desc'. Note that there are long-term plans to
get rid of IRQ numbers entirely, but those plans have existed for
a long time already without anybody seriously addressing the device
driver interfaces so far, so it might never really happen.
struct msi_ops {
struct msi_desc *(*msi_setup_entry)(struct msi_irqs *msi, struct
msi_desc *entry);
int msix_setup_entries(struct msi_irqs *msi, struct msix_entry
*entries);
u32 (*msi_mask_irq)(struct msi_desc *desc, u32 mask, u32 flag);
u32 (*msix_mask_irq)(struct msi_desc *desc, u32 flag);
void (*msi_read_message)(struct msi_desc *desc, struct msi_msg *msg);
void (*msi_write_message)(struct msi_desc *desc, struct msi_msg *msg);
void (*msi_set_intx)(struct msi_irqs *msi, int enable);
};
struct msi_ops provides several hook functions, generic MSI driver will call
the hook functions to access device specific registers. PCI devices will share
the same msi_ops, because they have the same way to access MSI hardware
registers.
Generic MSI layer export msi_capability_init() and msix_capability_init()
functions
to drivers. msi/x_capability_init() will initialize MSI capability data
struct msi_desc
and alloc the irq, then write the msi address/data value to hardware
registers.
This series only did compile test, we will test it in x86 and arm platform
later.
For the generic drivers, I don't see much point in differentiating between
MSI and MSI-X, as I believe the difference is something internal to the PCI
implementation.
With the other operations, I think they should all take a 'struct device *'
as the first argument for convenience and consistency. I don't think you
actually
need msi_read_message(), and we could avoid msi_write_message() by doing it
the other way round.
What I'd envision as the API from the device driver perspective is something
as simple like this:
struct msi_desc *msi_request(struct msi_chip *chip, irq_handler_t handler,
unsigned long flags, const char *name, struct device
*dev);
which would get an msi descriptor that is valid for this device (dev)
connected to a particular msi_chip, and associate a handler function
with it. The device driver can call that function and retrieve the
address/message pair from the msi_desc in order to store it in its own
device specific registers. The request_irq() can be handled internally
to msi_request().
Would that work for you?
Arnd
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC PATCH 00/11] Refactor MSI to support Non-PCI device
On 2014/7/29 22:08, Arnd Bergmann wrote:
On Saturday 26 July 2014 11:08:37 Yijing Wang wrote:
The series is a draft of generic MSI driver that supports PCI
and Non-PCI device which have MSI capability. If you're not interested
it, sorry for the noise.
I've finally managed to take some time to look at the series. Overall,
the concept looks good to me, and the patches look very well implemented.
The part I'm not sure about is the interface we want to end up with
at the end of the series. More on that below
Hi Arnd,
Thanks for your review and comments very much!
Please refer the inline comments.
The series is based on Linux-3.16-rc1.
MSI was introduced in PCI Spec 2.2. Currently, kernel MSI
driver codes are bonding with PCI device. Because MSI has a lot
advantages in design. More and more non-PCI devices want to
use MSI as their default interrupt. The existing MSI device
include HPET. HPET driver provide its own MSI code to initialize
and process MSI interrupts. In the latest GIC v3 spec, legacy device
can deliver MSI by the help of a relay device named consolidator.
Consolidator can translate the legacy interrupts connected to it
to MSI/MSI-X. And new non-PCI device will be designed to
support MSI in future. So make the MSI driver code be generic will
help the non-PCI device use MSI more simply.
The new data struct for generic MSI driver.
struct msi_irqs {
u8 msi_enabled:1; /* Enable flag */
u8 msix_enabled:1;
struct list_head msi_list; /* MSI desc list */
void *data; /* help to find the MSI device */
struct msi_ops *ops; /* MSI device specific hook */
};
struct msi_irqs is used to manage MSI related informations. Every device
supports
MSI should contain this data struct and allocate it.
I think you should have a stronger association with the 'struct
device' here. Can you replace the 'void *data' with 'struct device *dev'?
Actually, I used the struct device *dev in my first draft, finally, I replaced
it with void *data, because some MSI devices don't have a struct device *dev,
like the existing hpet device, dmar msi device, and OF device, like the ARM
consolidator.
Of course, we can make the MSI devices have their own struct device, and
register to
device tree, eg, add a class device named MSI_DEV. But I'm not sure whether it
is appropriate.
The other part I'm not completely sure about is how you want to
have MSIs map into normal IRQ descriptors. At the moment, all
MSI users are based on IRQ numbers, but this has known scalability problems.
Hmmm, I still use the IRQ number to map the MSIs to IRQ description.
I'm sorry, I don't understand you meaning.
What are the scalability problems you mentioned ?
For device drivers, they always process interrupt in two steps.
If irq is the legacy interrupt, drivers will first
use the irq_of_parse_and_map() or pci_enable_device() to parse and get the IRQ
number.
Then drivers will call the request_irq() to register the interrupt handler.
If irq is MSIs, first call pci_enable_msi/x() to get the IRQ number and then
call
request_irq() to register interrupt handler.
I wonder if we can do the interface in a way that
hides the interrupt number from generic device drivers and just
passes a 'struct irq_desc'. Note that there are long-term plans to
get rid of IRQ numbers entirely, but those plans have existed for
a long time already without anybody seriously addressing the device
driver interfaces so far, so it might never really happen.
Maybe this is a huge work, now hundreds drivers use the IRQ number, so maybe we
can consider
this in a separate title.
struct msi_ops {
struct msi_desc *(*msi_setup_entry)(struct msi_irqs *msi, struct
msi_desc *entry);
int msix_setup_entries(struct msi_irqs *msi, struct msix_entry
*entries);
u32 (*msi_mask_irq)(struct msi_desc *desc, u32 mask, u32 flag);
u32 (*msix_mask_irq)(struct msi_desc *desc, u32 flag);
void (*msi_read_message)(struct msi_desc *desc, struct msi_msg *msg);
void (*msi_write_message)(struct msi_desc *desc, struct msi_msg
*msg);
void (*msi_set_intx)(struct msi_irqs *msi, int enable);
};
struct msi_ops provides several hook functions, generic MSI driver will call
the hook functions to access device specific registers. PCI devices will
share
the same msi_ops, because they have the same way to access MSI hardware
registers.
Generic MSI layer export msi_capability_init() and msix_capability_init()
functions
to drivers. msi/x_capability_init() will initialize MSI capability data
struct msi_desc
and alloc the irq, then write the msi address/data value to hardware
registers.
This series only did compile test, we will test it in x86 and arm platform
later.
For the generic drivers, I don't see much point in differentiating between
MSI and MSI-X, as I believe the difference is something internal to the PCI
