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