On Tue, Jan 18, 2022 at 11:12:43AM +0800, Zhu, Lingshan wrote: > > > On 1/18/2022 11:01 AM, Jason Wang wrote: > > On Mon, Jan 17, 2022 at 3:57 PM Michael S. Tsirkin <[email protected]> > wrote: > > On Mon, Jan 17, 2022 at 02:15:55PM +0800, Jason Wang wrote: > > 在 2022/1/14 下午9:39, Michael S. Tsirkin 写道: > > The spec says (v1.1 4.1.4.5 ISR status capability): > > The VIRTIO_PCI_CAP_ISR_CFG capability refers to at least a > single byte, which contains the 8bit ISR > status field to be used for INT#x interrupt handling. > > and > > to avoid an extra access, simply reading this register resets > it to 0 and causes the device to deassert the > interrupt. > In this way, driver read of ISR status causes the device to > deassert an interrupt. > > See sections 4.1.5.3 and 4.1.5.4 for how this is used. > > and in 4.1.5.4 Notification of Device Configuration Changes > > it says: > > • If MSIX capability is disabled: > 1. Set the second lower bit of the ISR Status field for the > device. > 2. Send the appropriate PCI interrupt for the device. > > If MSIX capability is enabled: > 1. If config_msix_vector is not NO_VECTOR, request the > appropriate MSIX interrupt message for > the device, config_msix_vector sets the MSIX Table entry > number. > > all of the above make it looks like VIRTIO_PCI_CAP_ISR_CFG > capability is > unused with MSIX. > > This was actually the way the spec was understood by > Zhu Lingshan from Intel (Cc'd). > > However, looking at the conformance statements, one finds out > this is > not the case: > > 4.1.4.5.1 Device Requirements: ISR status capability > > > The device MUST present at least one VIRTIO_PCI_CAP_ISR_CFG > capability. > The device MUST set the Device Configuration Interrupt bit in > ISR status before sending a device configu > ration change notification to the driver. > If MSIX capability is disabled, the device MUST set the > Queue Interrupt bit in ISR status before sending a > virtqueue notification to the driver. > > which implies that the Device Configuration Interrupt bit is > set unconditionally. > > > > It is unfortunate that it does not copy this requirement in > more places, > and that the non-conformance text is incomplete and does not > mention the MSI-X usage at all. > > I propose to extend 4.1.4.5 ISR status capability and > 4.1.5.4 Notification of Device Configuration Changes > to mention the MSI use. > > I agree to expand ISR cap usage to MSI(MSIX) usage with clear descriptions, > and remove the limitations to MSIX. E.g,: > 4.1.4.5.2 Driver Requirements: ISR status capability > If MSI-X capability is enabled, the driver SHOULD NOT access ISR status upon > detecting a Queue Interrupt. > > Thanks >
No, we actually need that for bypass. Only config interrupts set ISR. I will need to send a patch, people are still confused. > I wonder do we want > > 1) mandate ISR bit > > or > > 2) remove the ISR bit set for MSI mode? > > 1) is the current Qemu behavior but seems a little bit contradict > with the > goal of MSI. > > Thanks > > > > I think we want the ISR bit > > This means there's no chance to use fast irq path but since it's a > less frequent operation. It should be fine. > > Thanks > > > since otherwise we need to go read > a ton of config space fields to check whether anything changed. > > -- > MST > > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
