On Tue, Sep 08, 2020 at 12:34:59PM +0200, Philippe Mathieu-Daudé wrote:
> 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.boddu@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.

Yes, yes it should.


> 
> > 
> > So no change needed for this patch.
> > 
> >>
> >> Regards,
> >> Sai Pavan
> >>>
> >>> With that fixed:
> >>> Reviewed-by: Philippe Mathieu-Daudé <philmd@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;
> >>>>
> >>
> > 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson