Re: [PATCH] hw/arm/smmuv3: Support non-PCI/PCIe devices connection

[Peter Maydell]

On Fri, 20 Aug 2021 at 10:04, Li, Chunming <Chunming.Li@verisilicon.com> wrote:
>
> > On Fri, 20 Aug 2021 at 03:36, Li, Chunming
> > <Chunming.Li@verisilicon.com> wrote:
> > >
> > > The current SMMU V3 device model only support PCI/PCIe devices,
> > > so we update it to support non-PCI/PCIe devices.
> > >
> > >     hw/arm/smmuv3:
> > >         . Create IOMMU memory regions for non-PCI/PCIe devices based
> > on their SID
> > >         . Add sid-map property to store non-PCI/PCIe devices SID
> > >         . Update implementation of CFGI commands based on device SID
> > >     hw/arm/smmu-common:
> > >         . Differentiate PCI/PCIe and non-PCI/PCIe devices SID getting
> > strategy
> > >     hw/arm/virt:
> > >         . Add PL330 DMA controller and connect with SMMUv3 for
> > testing
> > >         . Add smmuv3_sidmap for non-PCI/PCIe devices SID setting
> >
> > Please don't try to do all these things in one big patch --
> > put together a patchseries with several smaller patches,
> > each of which does one self-contained thing.
> >
>   Got it. Thanks for your comments.
>   Let me try to split them into several smaller patches.
>   But all these updates work for 1 thing. They are depend on each other.

Yes, that's why you send a single patch *series*, which has
a cover letter that explains the overall purpose. Then each
individual patch in the series has a commit message that
describes what that specific patch is doing. As emails, the
patches are all sent as follow-ups to the coverletter.
"git format-patch" and "git send-email" should handle this for you.

https://wiki.qemu.org/Contribute/SubmitAPatch#Split_up_long_patches
has a little more discussion of this.

> > If you have a patch that depends on another, it's usually better to
> > send them as a patchseries. I was wondering what the reason for
> > that PL330 patch was...

>   I need a non-PCI/PCIe device to do the verification. The old PL330 DMA 
> controller
>   cannot configure its memory region manually. So we update it and provide 
> path.
>   PL330 patch was reviewed and will be merged in target-arm.next for 6.2.
>   The virt.c compile will fail if there is no PL330 device patch.

Yeah, I accepted it because it is a sensible cleanup on its own;
but it would have been a bit easier for me to understand why you
were doing that if I'd had the wider context.

thanks
-- PMM