All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-block@nongnu.org,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	qemu-devel@nongnu.org, "Alexander Bulekov" <alxndr@bu.edu>,
	"Kevin O'Connor" <kevin@koconnor.net>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: Re: [RFC PATCH 3/6] hw/sd/sdcard: Do not use legal address '0' for INVALID_ADDRESS
Date: Mon, 21 Sep 2020 21:23:03 +0200	[thread overview]
Message-ID: <CABgObfbVsN-eh0eb3QUuk1qQ2mU92y80EjP-PVi-oV=mz0UAnw@mail.gmail.com> (raw)
In-Reply-To: <87363byxp0.fsf@dusky.pond.sub.org>

[-- Attachment #1: Type: text/plain, Size: 3144 bytes --]

I think we can just bite the bullet and bump the version number. Just like
not all boards are created equal in terms of migration compatibility,
neither are all devices.

Unfortunately pflash is among those that need some care, but we have much
more leeway with sdhci-pci.

Paolo

Il lun 21 set 2020, 17:08 Markus Armbruster <armbru@redhat.com> ha scritto:

> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>
> > On 9/21/20 2:24 PM, Dr. David Alan Gilbert wrote:
> >> * Markus Armbruster (armbru@redhat.com) wrote:
> >>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> >>>
> >>>> +Paolo & Kevin.
> >>>>
> >>>> On 9/21/20 10:40 AM, Markus Armbruster wrote:
> >>>>> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> >>>>>
> >>>>>> As it is legal to WRITE/ERASE the address/block 0,
> >>>>>> change the value of this definition to an illegal
> >>>>>> address: UINT32_MAX.
> >>>>>>
> >>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >>>>>> ---
> >>>>>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >>>>>> Cc: Markus Armbruster <armbru@redhat.com>
> >>>>>>
> >>>>>> Same problem I had with the pflash device last year...
> >>>>>> This break migration :(
> >>>>>> What is the best way to do this?
> >>>>>
> >>>>> Remind me: did we solve the problem with pflash, and if yes, how?
> >>>>
> >>>> No we can't. The best I could do is add a comment and as this
> >>>> is not fixable. See commit aba53a12bd5: ("hw/block/pflash_cfi01:
> >>>> Document use of non-CFI compliant command '0x00'").
> >>>>
> >>>> I now consider the device in maintenance-only
> >>>> mode and won't add any new features.
> >>>>
> >>>> I started working on a new implementation, hoping it can be a
> >>>> drop in replacement. Laszlo still has hope that QEMU pflash
> >>>> device will support sector locking so firmware developers could
> >>>> test upgrading fw in VMs.
> >>>>
> >>>> Back to the SDcard, it might be less critical, so a migration
> >>>> breaking change might be acceptable. I'm only aware of Paolo
> >>>> and Kevin using this device for testing. Not sure of its
> >>>> importance in production.
> >>>
> >>> Neither am I.
> >>>
> >>> Which machine types include this device by default?
> >>
> >> To me it looks like it's some of the ARM boards.
> >
> > My worry is TYPE_PCI_SDHCI ("sdhci-pci"):
> >
> >     k->vendor_id = PCI_VENDOR_ID_REDHAT;
> >     k->device_id = PCI_DEVICE_ID_REDHAT_SDHCI;
> >     k->class_id = PCI_CLASS_SYSTEM_SDHCI;
> >
> > config SDHCI_PCI
> >     bool
> >     default y if PCI_DEVICES
>
> Ah, now I remember.  Not the first time I wished it wouldn't exist...
>
> >>> How can a non-default device be added, and to which machine types?
> >>>
> >>> I gather the fix changes device state incompatibly.  Always, or only in
> >>> certain states?
>
> I think we need to answer this question.
>
> >>>                  I'm asking because if device state remains compatible
> >>> most of the time, we might be able use subsection trickery to keep
> >>> migration working most of the time.  Has been done before, I think.
>
>

[-- Attachment #2: Type: text/html, Size: 4946 bytes --]

  reply	other threads:[~2020-09-21 19:29 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-18 17:41 [RFC PATCH 0/6] hw/sd/sdcard: Do not attempt to erase out of range addresses Philippe Mathieu-Daudé
2020-09-18 17:41 ` [RFC PATCH 1/6] hw/sd/sdcard: Add trace event for ERASE command (CMD38) Philippe Mathieu-Daudé
2020-09-18 17:41 ` [RFC PATCH 2/6] hw/sd/sdcard: Introduce the INVALID_ADDRESS definition Philippe Mathieu-Daudé
2020-09-18 17:41 ` [RFC PATCH 3/6] hw/sd/sdcard: Do not use legal address '0' for INVALID_ADDRESS Philippe Mathieu-Daudé
2020-09-21  8:40   ` Markus Armbruster
2020-09-21 10:31     ` Philippe Mathieu-Daudé
2020-09-21 12:21       ` Markus Armbruster
2020-09-21 12:24         ` Dr. David Alan Gilbert
2020-09-21 14:23           ` Philippe Mathieu-Daudé
2020-09-21 15:08             ` Markus Armbruster
2020-09-21 19:23               ` Paolo Bonzini [this message]
2020-09-22 14:48                 ` Markus Armbruster
2020-10-15  6:27                   ` Philippe Mathieu-Daudé
2020-09-21 23:53       ` Kevin O'Connor
2020-09-18 17:41 ` [RFC PATCH 4/6] hw/sd/sdcard: Reset both start/end addresses on error Philippe Mathieu-Daudé
2020-09-18 17:41 ` [RFC PATCH 5/6] hw/sd/sdcard: Do not attempt to erase out of range addresses Philippe Mathieu-Daudé
2020-09-18 17:41 ` [RFC PATCH 6/6] hw/sd/sdcard: Assert if accessing an illegal group Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABgObfbVsN-eh0eb3QUuk1qQ2mU92y80EjP-PVi-oV=mz0UAnw@mail.gmail.com' \
    --to=pbonzini@redhat.com \
    --cc=alxndr@bu.edu \
    --cc=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=kevin@koconnor.net \
    --cc=philmd@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.