Adding Paolo and Eduardo since we're wandering into QOM-land. Kevin Wolf <kw...@redhat.com> writes:
> Am 08.08.2022 um 08:26 hat Markus Armbruster geschrieben: >> Daniel P. Berrangé <berra...@redhat.com> writes: >> >> > On Thu, Aug 04, 2022 at 05:30:40PM +0200, Markus Armbruster wrote: >> >> Daniel P. Berrangé <berra...@redhat.com> writes: >> >> >> >> > On Thu, Aug 04, 2022 at 04:56:15PM +0200, Markus Armbruster wrote: >> >> >> Daniel P. Berrangé <berra...@redhat.com> writes: >> >> >> >> >> >> > On Thu, Jul 28, 2022 at 10:46:35AM +0100, Peter Maydell wrote: >> >> >> >> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kw...@redhat.com> wrote: >> >> >> >> > >> >> >> >> > Am 18.07.2022 um 11:49 hat Markus Armbruster geschrieben: >> >> >> >> > > An OTP device isn't really a parallel flash, and neither are >> >> >> >> > > eFuses. >> >> >> >> > > More fast-and-lose use of IF_PFLASH may exist in the tree, and >> >> >> >> > > maybe of >> >> >> >> > > other interface types, too. >> >> >> >> > > >> >> >> >> > > This patch introduces IF_OTHER. The patch after next uses it >> >> >> >> > > for an >> >> >> >> > > EEPROM device. >> >> >> >> > > >> >> >> >> > > Do we want IF_OTHER? >> >> >> >> > >> >> >> >> > What would the semantics even be? Any block device that doesn't >> >> >> >> > pick up >> >> >> >> > a different category may pick up IF_OTHER backends? >> >> >> >> > >> >> >> >> > It certainly feels like a strange interface to ask for "other" >> >> >> >> > disk and >> >> >> >> > then getting as surprise what this other thing might be. It's >> >> >> >> > essentially the same as having an explicit '-device other', and I >> >> >> >> > suppose most people would find that strange. >> >> >> >> > >> >> >> >> > > If no, I guess we get to abuse IF_PFLASH some more. >> >> >> >> > > >> >> >> >> > > If yes, I guess we should use IF_PFLASH only for actual >> >> >> >> > > parallel flash >> >> >> >> > > memory going forward. Cleaning up existing abuse of IF_PFLASH >> >> >> >> > > may not >> >> >> >> > > be worth the trouble, though. >> >> >> >> > > >> >> >> >> > > Thoughts? >> >> >> >> > >> >> >> >> > If the existing types aren't good enough (I don't have an opinion >> >> >> >> > on >> >> >> >> > whether IF_PFLASH is a good match), let's add a new one. But a >> >> >> >> > specific >> >> >> >> > new one, not just "other". >> >> >> >> >> >> >> >> I think the common thread is "this isn't what anybody actually >> >> >> >> thinks >> >> >> >> of as being a 'disk', but we would like to back it with a block >> >> >> >> device >> >> >> >> anyway". That can cover a fair range of possibilities... >> >> >> > >> >> >> > Given that, do we even want/have to use -drive for this ? We can >> >> >> > use >> >> >> > -blockdev for the backend and reference that from any -device we want >> >> >> > to create, and leave -drive out of the picture entirely >> >> >> >> >> >> -drive is our only means to configure onboard devices. >> >> >> >> >> >> We've talked about better means a few times, but no conclusions. I can >> >> >> dig up pointers, if you're interested. >> >> > >> >> > For onboard pflash with x86, we've just got properties against the >> >> > machine that we can point to a blockdev. >> >> >> >> True, but the vast majority of onboard block devices doesn't come with >> >> such properties. Please see >> >> >> >> Subject: On configuring onboard block devices with -blockdev (was: [PATCH >> >> v4 6/7] hw/nvram: Update at24c EEPROM init function in NPCM7xx boards) >> >> Date: Mon, 15 Nov 2021 16:28:33 +0100 >> >> Message-ID: <875ystigke.fsf...@dusky.pond.sub.org> >> >> https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg03173.html >> > >> > My take away from your mail there is that in the absence of better ideas >> > we should at least use machine properties for anything new we do so we >> > don't make the problem worse than it already is. It feels more useful >> > than inventing new IF_xxx possibilities for something we think is the >> > wrong approach already. >> >> The difficulty of providing machine properties for existing devices and >> the lack of adoption even for new devices make me doubt they are a >> viable path forward. In the message I referenced above, I wrote: >> >> If "replace them all by machine properties" is the way forward, we >> need to get going. At the current rate of one file a year (measured >> charitably), we'll be done around 2090, provided we don't add more >> (we've added quite a few since I did the first replacement). >> >> I figure this has slipped to the 22nd century by now. >> >> Yes, more uses of -drive are steps backward. But they are trivially >> easy, and also drops in the bucket. Machine properties are more >> difficult, and whether they actually take us forward seems doubtful. > > Machine properties are also not what we really want, but just a > workaround. How about implementing the real thing, providing qdev > properties for built-in devices? > > Looking at a few random users of drive_get(), they look like this: > > DriveInfo *dinfo = drive_get(...); > dev = qdev_new("driver-type"); > qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo)); > qdev_realize_and_unref(...); > > What if we assigned a name to every built-in device Before we continue: I like the idea of giving onboard devices user-friendly, stable names. "serial0" beats "/machine/unattached/device[13]" hands down. Even more so where the "device[13]" part depends on other options (e.g. with "-vga none" it's "device[12]", both for i440FX). The question is what these names could be. They can't be qdev IDs, because we blew our chance to reserve names. Could we solve this in QOM? > and had a > qdev_new_builtin(type, id) that applies qdev properties given on the > command line to the device? Could be either with plain old '-device' > (we're already doing a similar thing with default devices where adding > -device for the existing device removes the default device) or with a > new command line option. > > The qdev_prop_set_drive() would then become conditional and would only > be done if the property wasn't already set in qdev_new_builtin(). Or > maybe a new function that checks for conflicting -drive and -device. > Though that part is just implementation details. The idea sounds vaguely familiar. Whether it's because we discussed similar ones on the list, or because I mulled over similar ones in my head I can't say for sure. Overloading -device so it can also configure existing devices feels iffy. We already have means to set properties for (onboard) devices to pick up: -global. But it's per device *type*, and therefore falls apart as soon as you have more than one of the type. We need something that affects a specific (onboard) device, and for that we need a way to address one. The QOM paths we have don't cut it, as I illustrated above.
