On Thu, 29 Feb 2024 at 04:52, Joe Komlodi <koml...@google.com> wrote:
> On Wed, Feb 28, 2024 at 6:21 AM Peter Maydell <peter.mayd...@linaro.org> 
> wrote:
> > So as far as I can see, this patchset defines a bunch of mechanism,
> > but no actual users: no device looks at these new memattrs, no board
> > code sets the properties. I don't really want to add this without
> > an upstream usecase for it.
>
> Yeah, I believe the current use-cases for this series are mostly downstream.
> It's possible that there's an upstream device that might benefit from
> it, but I'm not aware of one.
>
> Is the concern the usefulness of the series, or the worry about it 
> bit-rotting?
> If it's the latter, would a qtest be alright to make sure it doesn't rot?

My main issues are:
 * it's hard to review design without the uses of the code
 * it's extra complexity and maintenance burden that we don't
   need (upstream): accepting the patches gives upstream extra
   work to deal with into the future with no benefit to us
 * dead code is code that typically we would remove
 * we end up with something we can't refactor or clean up
   or change because the only reason we have it is for code
   that we don't have any visibility into: effectively it
   becomes an API for us that we can't change, which is not
   something QEMU does except for specific well defined API
   surfaces (QMP, plugins, etc)

Our usual approach is "submit the patches that add the new core
feature/mechanism along with the patches that add the new
device/board/etc that uses it". Compare the recent patches
also from Google for the ITS and SMMU that try to add hooks
that aren't needed by anything in upstream QEMU:
https://patchew.org/QEMU/20240221171716.1260192-1-nabiheste...@google.com/20240221171716.1260192-3-nabiheste...@google.com/
https://patchew.org/QEMU/20240221173325.1494895-1-nabiheste...@google.com/20240221173325.1494895-3-nabiheste...@google.com/
-- we rejected those for the same reason.

thanks
-- PMM

Reply via email to