Re: [PATCH v5 5/8] blockdev: Add a new IF type IF_OTHER

From: "Cédric Le Goater" <clg@kaod.org>
To: Kevin Wolf <kwolf@redhat.com>, Peter Maydell <peter.maydell@linaro.org>
Cc: Markus Armbruster <armbru@redhat.com>,
	Hao Wu <wuhaotsh@google.com>, <richard.henderson@linaro.org>,
	<qemu-arm@nongnu.org>, <qemu-devel@nongnu.org>,
	<venture@google.com>, <Avi.Fishman@nuvoton.com>,
	<kfting@nuvoton.com>, <hskinnemoen@google.com>, <f4bug@amsat.org>,
	<bin.meng@windriver.com>, <qemu-block@nongnu.org>,
	<thuth@redhat.com>, Hanna Reitz <hreitz@redhat.com>
Subject: Re: [PATCH v5 5/8] blockdev: Add a new IF type IF_OTHER
Date: Thu, 28 Jul 2022 15:57:21 +0200	[thread overview]
Message-ID: <6799cfc6-fd72-4061-0707-e5089a9ee50d@kaod.org> (raw)
In-Reply-To: <YuKPVi9UjmZVqw5a@redhat.com>

On 7/28/22 15:29, Kevin Wolf wrote:
> Am 28.07.2022 um 11:46 hat Peter Maydell geschrieben:
>> On Wed, 27 Jul 2022 at 20:03, Kevin Wolf <kwolf@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...
> 
> How confident are we that no board will ever have two devices of this
> kind?

The BMC machines have a lot of eeproms.

> 
> As long as every board has at most one, if=other is a bad user interface
> in terms of descriptiveness, but still more or less workable as long as
> you know what it means for the specific board you use.
> 
> But if you have more than one device, it becomes hard to predict which
> device gets which backend - it depends on the initialisation order in
> the code then, and I'm pretty sure that this isn't something that should
> have significance in external interfaces and therefore become a stable
> API.

Can't we use the drive index ?

There has been various attempts to solve this problem for the Aspeed
machines also. See below. May be we should introduce and IF_EEPROM for
the purpose.

Thanks,

C.

[PATCH v2] aspeed: qcom: add block backed FRU devices
https://www.mail-archive.com/qemu-devel@nongnu.org/msg900485.html

[PATCH] aspeed: Enable backend file for eeprom
http://patchwork.ozlabs.org/project/qemu-devel/patch/20220728061228.152704-1-wangzhiqiang02@inspur.com/