On 2012-06-21 12:50, Alexey Kardashevskiy wrote:
> On 21/06/12 20:38, Jan Kiszka wrote:
>> On 2012-06-21 12:28, Alexey Kardashevskiy wrote:
>>> On 21/06/12 17:39, Jan Kiszka wrote:
>>>> On 2012-06-21 09:18, Alexey Kardashevskiy wrote:
>>>>>
>>>>> agrhhh. sha1 of the patch changed after rebasing :)
>>>>>
>>>>>
>>>>>
>>>>> Added (msi|msix)_(set|get)_message() function for whoever might
>>>>> want to use them.
>>>>>
>>>>> Currently msi_notify()/msix_notify() write to these vectors to
>>>>> signal the guest about an interrupt so the correct values have to
>>>>> written there by the guest or QEMU.
>>>>>
>>>>> For example, POWER guest never initializes MSI/MSIX vectors, instead
>>>>> it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
>>>>> POWER we have to initialize MSI/MSIX message from QEMU.
>>>>>
>>>>> As only set* function are required by now, the "get" functions were added
>>>>> or made public for a symmetry.
>>>>>
>>>>> Signed-off-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
>>>>> ---
>>>>> hw/msi.c | 29 +++++++++++++++++++++++++++++
>>>>> hw/msi.h | 2 ++
>>>>> hw/msix.c | 11 ++++++++++-
>>>>> hw/msix.h | 3 +++
>>>>> 4 files changed, 44 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/hw/msi.c b/hw/msi.c
>>>>> index 5233204..9ad84a4 100644
>>>>> --- a/hw/msi.c
>>>>> +++ b/hw/msi.c
>>>>> @@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const PCIDevice* dev, bool msi64bit)
>>>>> return dev->msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : PCI_MSI_PENDING_32);
>>>>> }
>>>>>
>>>>> +MSIMessage msi_get_message(PCIDevice *dev)
>>>>
>>>> MSIMessage msi_get_message(PCIDevice *dev, unsigned vector)
>>>
>>>
>>> Who/how/why is going to calculate the vector here?
>>>
>>>>
>>>>> +{
>>>>> + uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
>>>>> + bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
>>>>> + MSIMessage msg;
>>>>> +
>>>>> + if (msi64bit) {
>>>>> + msg.address = pci_get_quad(dev->config + msi_address_lo_off(dev));
>>>>> + } else {
>>>>> + msg.address = pci_get_long(dev->config + msi_address_lo_off(dev));
>>>>> + }
>>>>> + msg.data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));
>>>>
>>>> And I have this here in addition:
>>>>
>>>> unsigned int nr_vectors = msi_nr_vectors(flags);
>>>> ...
>>>>
>>>> if (nr_vectors > 1) {
>>>> msg.data &= ~(nr_vectors - 1);
>>>> msg.data |= vector;
>>>> }
>>>>
>>>> See PCI spec and existing code.
>>>
>>>
>>> What for? I really do not get it why someone might want to read something but not real value.
>>> What PCI code should I look?
>>
>> I'm not sure what your use case for reading the message is. For KVM
>> device assignment it is preparing an alternative message delivery path
>> for MSI vectors. And for this we will need vector notifier support for
>> MSI as well. You can check the MSI-X code for corresponding use cases of
>> msix_get_message.
>
>> And when we already have msi_get_message, another logical use case is
>> msi_notify. See msix.c again.
>
> Aaaa.
>
> I have no case for reading the message. All I need is writing. And I want it public as I want to use
> it from hw/spapr_pci.c. You suggested to add reading, I added "get" to be _symmetric_ to "set"
> ("get" returns what "set" wrote). You want a different thing which I can do but it is not
> msi_get_message(), it is something like msi_prepare_message(MSImessage msg) or
> msi_set_vector(uint16_t data) or simply internal kitchen of msi_notify().
>
> Still can do what you suggested, it just does not seem right.
It is right - when looking at it from a different angle. ;)
I don't mind if you add msi_get_message now or leave this to me. Likely
the latter is better as you have no use case for msi_get_message (and
also msix_get_message!) outside of their modules, thus we should not
export those functions anyway.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
