On 9/8/20 1:35 PM, Sai Pavan Boddu wrote:
> Hi Philippe,
>
>> -----Original Message-----
>> From: Philippe Mathieu-Daudé <phi...@redhat.com>
>> Sent: Tuesday, September 8, 2020 4:05 PM
>> To: Sai Pavan Boddu <saip...@xilinx.com>; Peter Maydell
>> <peter.mayd...@linaro.org>; Markus Armbruster <arm...@redhat.com>;
>> 'Marc-André Lureau' <marcandre.lur...@redhat.com>; Paolo Bonzini
>> <pbonz...@redhat.com>; Gerd Hoffmann <kra...@redhat.com>; Edgar
>> Iglesias <edg...@xilinx.com>; Francisco Eduardo Iglesias
>> <figle...@xilinx.com>; David Gibson <da...@gibson.dropbear.id.au>
>> Cc: qemu-devel@nongnu.org; Alistair Francis <alistair.fran...@wdc.com>;
>> Eduardo Habkost <ehabk...@redhat.com>; Ying Fang
>> <fangyi...@huawei.com>; Vikram Garhwal <f...@xilinx.com>; Paul
>> Zimmerman <pauld...@gmail.com>
>> Subject: Re: [PATCH v4 1/7] usb/hcd-xhci: Make dma read/writes hooks pci
>> free
>>
>> On 9/8/20 12:30 PM, Philippe Mathieu-Daudé wrote:
>>>>> On 8/28/20 9:19 PM, Sai Pavan Boddu wrote:
>>>>>> This patch starts making the hcd-xhci.c pci free, as part of this
>>>>>> restructuring dma read/writes are handled without passing pci object.
>>>>>>
>>>>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.bo...@xilinx.com>
>>>>>> ---
>>>>>> hw/usb/hcd-xhci.c | 24 +++++++++++------------- hw/usb/hcd-xhci.h
>>>>>> |
>>>>>> 3 +++
>>>>>> 2 files changed, 14 insertions(+), 13 deletions(-)
>>>>>>
>>> [...]
>>>>>> --- a/hw/usb/hcd-xhci.h
>>>>>> +++ b/hw/usb/hcd-xhci.h
>>>>>> @@ -22,6 +22,8 @@
>>>>>> #ifndef HW_USB_HCD_XHCI_H
>>>>>> #define HW_USB_HCD_XHCI_H
>>>>>>
>>>>>> +#include "sysemu/dma.h"
>>>>>
>>>>> AddressSpace is forward-declared in "qemu/typedefs.h", so no need to
>>>>> include it here (yes in the sources including hcd-xhci.h).
>>>> [Sai Pavan Boddu] Yes you are right!, but without this " dma_addr_t " is
>> undefined.
>>>> At this point of the patch, hcd-xhci.h is compiled along with pci.h which
>> would provide 'dma_addr_t', but when we strip the pci wrapper around hcd-
>> xhci we would miss it. Let me know, if its good to add later in the patch
>> series
>> when the split happens.
>>>
>>> OK :( I'd prefer to only include "sysemu/dma.h" in hw/usb/hcd-xhci.c.
>>>
>>> Cc'ing David who added dma_addr_t in commit e5332e6334f
>>> ("iommu: Introduce IOMMU emulation infrastructure"), it might be
>>> simpler to move its declaration to "qemu/typedefs.h"
>>> too.
>>
>> Btw this type should be poisoned for user-mode.
> [Sai Pavan Boddu] Thanks, I would try to define them in typedefs.h.
> BTW I'm not clear about poisoning then in user-mode. Do you mean hide these
> defines for user-mode ?
This doesn't "hide" but poison them: if you try to use them the
compiler (cpp actually) will raise an error.
This was not a direct comment for you, more something to discuss
out of the scope of your series ;) No need to try to declare it
poisoned now.
>
> Regards
> Sai Pavan
>>
>>>
>>> So no change needed for this patch.
>>>
>>>>
>>>> Regards,
>>>> Sai Pavan
>>>>>
>>>>> With that fixed:
>>>>> Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>>>>>
>>>>>> +
>>>>>> #define TYPE_XHCI "base-xhci"
>>>>>> #define TYPE_NEC_XHCI "nec-usb-xhci"
>>>>>> #define TYPE_QEMU_XHCI "qemu-xhci"
>>>>>> @@ -189,6 +191,7 @@ struct XHCIState {
>>>>>>
>>>>>> USBBus bus;
>>>>>> MemoryRegion mem;
>>>>>> + AddressSpace *as;
>>>>>> MemoryRegion mem_cap;
>>>>>> MemoryRegion mem_oper;
>>>>>> MemoryRegion mem_runtime;
>>>>>>
>>>>
>>>
>
