[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: does drive_get_next(IF_NONE) make sense?

From: Markus Armbruster
Subject: Re: does drive_get_next(IF_NONE) make sense?
Date: Wed, 03 Nov 2021 09:41:19 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux)

Peter Maydell <peter.maydell@linaro.org> writes:

> Does it make sense for a device/board to do drive_get_next(IF_NONE) ?

Short answer: hell, no!  ;)

Long answer below.

> At the moment we have exactly one user of this, which is
> hw/misc/sifive_u_otp.c. This is a model of a one-time-programmable
> fuse, and the drive is providing the backing store for the fuse
> contents. Borrowing an IF_NONE for this seems a bit odd, but
> it's not clear any of the other IF_ types is better.
> We also just (this release cycle) added models of the Xilinx
> efuse OTP fuses. Those have been implemented to use IF_PFLASH.
> (This is a somewhat unfortunate inconsistency I guess.)
> We also have a patchseries currently in the code review stage
> which uses IF_NONE:
> 20211101232346.1070813-1-wuhaotsh@google.com/20211101232346.1070813-6-wuhaotsh@google.com/">https://patchew.org/QEMU/20211101232346.1070813-1-wuhaotsh@google.com/20211101232346.1070813-6-wuhaotsh@google.com/
> Here we are trying to provide a drive as backing store for some
> EEPROMs that hang off the i2c buses on some npcm7xx boards.
> Are these uses of IF_NONE OK, or should we be doing something
> else (using IF_PFLASH, defining a new IF_*, ???)

Two questions, really: one, may boards IF_NONE for onboard devices, and
two, should new board code use drive_get_next().

drive_get_next() is basically a bad idea.  It returns the "next" block
backend of a certain interface type.  "Next" means bus=0,unit=N, where
subsequent calls count N up from zero, per interface type.

This lets you define unit numbers implicitly by execution order.  If the
order changes, or new calls appear "in the middle", unit numbers change.
ABI break.  Hard to spot in review.

Old hat: explicit is better than implicit.  Better use drive_get().
Even if that means you have to pass unit numbers around.

IF_NONE is *only* for use with -device.

Traditional -drive (before IF_NONE) does two things:

(1) Create a block backend device

(2) Request the board to create a block frontend device

    Which kind of device the board creates is up to the board.  Nothing
    but common decency stops a board from creating a floppy when asked
    for an IF_PFLASH.

When -device was invented, we needed a way to create just a block

A working (at the time), but obviously bad way was to use -drive to
create one the board ignores.

Improvement: invent an interface type all the boards ignore.  This is

Hindsight: we should have created a separate option instead of
overloading -drive.  Such an option now exists: -blockdev.

The difference between IF_NONE and the other interface types is more
than just convention: we actually detect when the board ignores a
request to create a block device, like this:

    $ qemu-system-x86_64 -S -drive if=mtd
    qemu-system-x86_64: -drive if=mtd: machine type does not support 

We don't do this for if=none, because those may still be used with

    $ qemu-system-x86_64 -nodefaults -display none -S -drive if=none,id=mt 
-device virtio-scsi -monitor stdio
    QEMU 6.1.50 monitor - type 'help' for more information
    (qemu) device_add scsi-cd,drive=mt,id=cd0
    (qemu) info block
    mt: [not inserted]
        Attached to:      cd0
        Removable device: not locked, tray closed

Therefore, having the board use IF_NONE just like a traditional
interface type is *wrong*.

As I explained above, the board can use any traditional interface type.
If none of them matches, and common decency prevents use of a
non-matching one, invent a new one.  We could do IF_OTHER.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]