1. Patchew
  2. QEMU
  3. [PATCH v2 00/22] eMMC support
  4. View patch

[PATCH v2 01/22] block: add eMMC block device type

Sai Pavan Boddu posted 22 patches 4 years, 10 months ago
There is a newer version of this series
[PATCH v2 01/22] block: add eMMC block device type
Posted by Sai Pavan Boddu 4 years, 10 months ago 
From: Vincent Palatin <vpalatin@chromium.org>

Add new block device type.

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
[SPB: Rebased over 5.1 version]
Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 include/sysemu/blockdev.h | 1 +
 blockdev.c                | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 3b5fcda..eefae9f 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -24,6 +24,7 @@ typedef enum {
      */
     IF_NONE = 0,
     IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
+    IF_EMMC,
     IF_COUNT
 } BlockInterfaceType;
 
diff --git a/blockdev.c b/blockdev.c
index cd438e6..390d43c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
     [IF_SD] = "sd",
     [IF_VIRTIO] = "virtio",
     [IF_XEN] = "xen",
+    [IF_EMMC] = "emmc",
 };
 
 static int if_max_devs[IF_COUNT] = {
-- 
2.7.4
Re: [PATCH v2 01/22] block: add eMMC block device type
Posted by Philippe Mathieu-Daudé 4 years, 10 months ago 
On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> From: Vincent Palatin <vpalatin@chromium.org>
> 
> Add new block device type.
> 
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> [SPB: Rebased over 5.1 version]
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  include/sysemu/blockdev.h | 1 +
>  blockdev.c                | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> index 3b5fcda..eefae9f 100644
> --- a/include/sysemu/blockdev.h
> +++ b/include/sysemu/blockdev.h
> @@ -24,6 +24,7 @@ typedef enum {
>       */
>      IF_NONE = 0,
>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
> +    IF_EMMC,
>      IF_COUNT
>  } BlockInterfaceType;
>  
> diff --git a/blockdev.c b/blockdev.c
> index cd438e6..390d43c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
>      [IF_SD] = "sd",
>      [IF_VIRTIO] = "virtio",
>      [IF_XEN] = "xen",
> +    [IF_EMMC] = "emmc",
>  };

We don't need to introduce support for the legacy -drive magic.

-device should be enough for this device, right?
Re: [PATCH v2 01/22] block: add eMMC block device type
Posted by Markus Armbruster 4 years, 10 months ago 
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
>> From: Vincent Palatin <vpalatin@chromium.org>
>> 
>> Add new block device type.
>> 
>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>> [SPB: Rebased over 5.1 version]
>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>  include/sysemu/blockdev.h | 1 +
>>  blockdev.c                | 1 +
>>  2 files changed, 2 insertions(+)
>> 
>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
>> index 3b5fcda..eefae9f 100644
>> --- a/include/sysemu/blockdev.h
>> +++ b/include/sysemu/blockdev.h
>> @@ -24,6 +24,7 @@ typedef enum {
>>       */
>>      IF_NONE = 0,
>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
>> +    IF_EMMC,
>>      IF_COUNT
>>  } BlockInterfaceType;
>>  
>> diff --git a/blockdev.c b/blockdev.c
>> index cd438e6..390d43c 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
>>      [IF_SD] = "sd",
>>      [IF_VIRTIO] = "virtio",
>>      [IF_XEN] = "xen",
>> +    [IF_EMMC] = "emmc",
>>  };
>
> We don't need to introduce support for the legacy -drive magic.
>
> -device should be enough for this device, right?

External interface extensions need rationale: why do we want / need it?
The commit message neglects to provide one.

Even more so when the interface in question is in a state like -drive
is.
Re: [PATCH v2 01/22] block: add eMMC block device type
Posted by Dr. David Alan Gilbert 4 years, 10 months ago 
* Markus Armbruster (armbru@redhat.com) wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
> > On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> >> From: Vincent Palatin <vpalatin@chromium.org>
> >> 
> >> Add new block device type.
> >> 
> >> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> >> [SPB: Rebased over 5.1 version]
> >> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> >> Signed-off-by: Joel Stanley <joel@jms.id.au>
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >> ---
> >>  include/sysemu/blockdev.h | 1 +
> >>  blockdev.c                | 1 +
> >>  2 files changed, 2 insertions(+)
> >> 
> >> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> >> index 3b5fcda..eefae9f 100644
> >> --- a/include/sysemu/blockdev.h
> >> +++ b/include/sysemu/blockdev.h
> >> @@ -24,6 +24,7 @@ typedef enum {
> >>       */
> >>      IF_NONE = 0,
> >>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
> >> +    IF_EMMC,
> >>      IF_COUNT
> >>  } BlockInterfaceType;
> >>  
> >> diff --git a/blockdev.c b/blockdev.c
> >> index cd438e6..390d43c 100644
> >> --- a/blockdev.c
> >> +++ b/blockdev.c
> >> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> >>      [IF_SD] = "sd",
> >>      [IF_VIRTIO] = "virtio",
> >>      [IF_XEN] = "xen",
> >> +    [IF_EMMC] = "emmc",
> >>  };
> >
> > We don't need to introduce support for the legacy -drive magic.
> >
> > -device should be enough for this device, right?
> 
> External interface extensions need rationale: why do we want / need it?
> The commit message neglects to provide one.
> 
> Even more so when the interface in question is in a state like -drive
> is.

I wouldn't be too nasty about -drive;  for me I still find it the
easiest way to start a VM.

Dave

-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [PATCH v2 01/22] block: add eMMC block device type
Posted by Philippe Mathieu-Daudé 4 years, 10 months ago 
On 2/22/21 2:16 PM, Dr. David Alan Gilbert wrote:
> * Markus Armbruster (armbru@redhat.com) wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>>
>>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
>>>> From: Vincent Palatin <vpalatin@chromium.org>
>>>>
>>>> Add new block device type.
>>>>
>>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>>>> [SPB: Rebased over 5.1 version]
>>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>> ---
>>>>  include/sysemu/blockdev.h | 1 +
>>>>  blockdev.c                | 1 +
>>>>  2 files changed, 2 insertions(+)
>>>>
>>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
>>>> index 3b5fcda..eefae9f 100644
>>>> --- a/include/sysemu/blockdev.h
>>>> +++ b/include/sysemu/blockdev.h
>>>> @@ -24,6 +24,7 @@ typedef enum {
>>>>       */
>>>>      IF_NONE = 0,
>>>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN,
>>>> +    IF_EMMC,
>>>>      IF_COUNT
>>>>  } BlockInterfaceType;
>>>>  
>>>> diff --git a/blockdev.c b/blockdev.c
>>>> index cd438e6..390d43c 100644
>>>> --- a/blockdev.c
>>>> +++ b/blockdev.c
>>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
>>>>      [IF_SD] = "sd",
>>>>      [IF_VIRTIO] = "virtio",
>>>>      [IF_XEN] = "xen",
>>>> +    [IF_EMMC] = "emmc",
>>>>  };
>>>
>>> We don't need to introduce support for the legacy -drive magic.
>>>
>>> -device should be enough for this device, right?
>>
>> External interface extensions need rationale: why do we want / need it?
>> The commit message neglects to provide one.
>>
>> Even more so when the interface in question is in a state like -drive
>> is.
> 
> I wouldn't be too nasty about -drive;  for me I still find it the
> easiest way to start a VM.

But eMMC isn't a bus where you can plug drives, it is soldered
on-board and is mmio mapped to a fixed address. I don't see the
point of having a drive interface for it...
RE: [PATCH v2 01/22] block: add eMMC block device type
Posted by Sai Pavan Boddu 4 years, 9 months ago 
Hi Philippe,

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> Sent: Monday, February 22, 2021 6:54 PM
> To: Dr. David Alan Gilbert <dgilbert@redhat.com>; Markus Armbruster
> <armbru@redhat.com>
> Cc: Sai Pavan Boddu <saipava@xilinx.com>; Kevin Wolf <kwolf@redhat.com>;
> Max Reitz <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent Palatin
> <vpalatin@chromium.org>; Thomas Huth <thuth@redhat.com>; Stefan
> Hajnoczi <stefanha@redhat.com>; Peter Maydell <peter.maydell@linaro.org>;
> Alistair Francis <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>;
> Luc Michel <luc.michel@greensocs.com>; Paolo Bonzini
> <pbonzini@redhat.com>; Sai Pavan Boddu <saipava@xilinx.com>; qemu-
> devel@nongnu.org; qemu-block@nongnu.org
> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> 
> On 2/22/21 2:16 PM, Dr. David Alan Gilbert wrote:
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> >>
> >>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> >>>> From: Vincent Palatin <vpalatin@chromium.org>
> >>>>
> >>>> Add new block device type.
> >>>>
> >>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> >>>> [SPB: Rebased over 5.1 version]
> >>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> >>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >>>> ---
> >>>>  include/sysemu/blockdev.h | 1 +
> >>>>  blockdev.c                | 1 +
> >>>>  2 files changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> >>>> index 3b5fcda..eefae9f 100644
> >>>> --- a/include/sysemu/blockdev.h
> >>>> +++ b/include/sysemu/blockdev.h
> >>>> @@ -24,6 +24,7 @@ typedef enum {
> >>>>       */
> >>>>      IF_NONE = 0,
> >>>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD,
> >>>> IF_VIRTIO, IF_XEN,
> >>>> +    IF_EMMC,
> >>>>      IF_COUNT
> >>>>  } BlockInterfaceType;
> >>>>
> >>>> diff --git a/blockdev.c b/blockdev.c index cd438e6..390d43c 100644
> >>>> --- a/blockdev.c
> >>>> +++ b/blockdev.c
> >>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> >>>>      [IF_SD] = "sd",
> >>>>      [IF_VIRTIO] = "virtio",
> >>>>      [IF_XEN] = "xen",
> >>>> +    [IF_EMMC] = "emmc",
> >>>>  };
> >>>
> >>> We don't need to introduce support for the legacy -drive magic.
> >>>
> >>> -device should be enough for this device, right?
> >>
> >> External interface extensions need rationale: why do we want / need it?
> >> The commit message neglects to provide one.
> >>
> >> Even more so when the interface in question is in a state like -drive
> >> is.
> >
> > I wouldn't be too nasty about -drive;  for me I still find it the
> > easiest way to start a VM.
> 
> But eMMC isn't a bus where you can plug drives, it is soldered on-board and is
> mmio mapped to a fixed address. I don't see the point of having a drive
> interface for it...
[Sai Pavan Boddu] Yeah, this makes sense but having a drive would be a simple implementation without disturbing much in the sd card emulation code. And its just easy to use, just as how sd cards are inserted.

I need to see, how easy it would be with -device.

Thanks,
Sai Pavan
Re: [PATCH v2 01/22] block: add eMMC block device type
Posted by Dr. David Alan Gilbert 4 years, 9 months ago 
* Sai Pavan Boddu (saipava@xilinx.com) wrote:
> Hi Philippe,
> 
> > -----Original Message-----
> > From: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Sent: Monday, February 22, 2021 6:54 PM
> > To: Dr. David Alan Gilbert <dgilbert@redhat.com>; Markus Armbruster
> > <armbru@redhat.com>
> > Cc: Sai Pavan Boddu <saipava@xilinx.com>; Kevin Wolf <kwolf@redhat.com>;
> > Max Reitz <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> > <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> > <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent Palatin
> > <vpalatin@chromium.org>; Thomas Huth <thuth@redhat.com>; Stefan
> > Hajnoczi <stefanha@redhat.com>; Peter Maydell <peter.maydell@linaro.org>;
> > Alistair Francis <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>;
> > Luc Michel <luc.michel@greensocs.com>; Paolo Bonzini
> > <pbonzini@redhat.com>; Sai Pavan Boddu <saipava@xilinx.com>; qemu-
> > devel@nongnu.org; qemu-block@nongnu.org
> > Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> > 
> > On 2/22/21 2:16 PM, Dr. David Alan Gilbert wrote:
> > > * Markus Armbruster (armbru@redhat.com) wrote:
> > >> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> > >>
> > >>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> > >>>> From: Vincent Palatin <vpalatin@chromium.org>
> > >>>>
> > >>>> Add new block device type.
> > >>>>
> > >>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> > >>>> [SPB: Rebased over 5.1 version]
> > >>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > >>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
> > >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > >>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > >>>> ---
> > >>>>  include/sysemu/blockdev.h | 1 +
> > >>>>  blockdev.c                | 1 +
> > >>>>  2 files changed, 2 insertions(+)
> > >>>>
> > >>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> > >>>> index 3b5fcda..eefae9f 100644
> > >>>> --- a/include/sysemu/blockdev.h
> > >>>> +++ b/include/sysemu/blockdev.h
> > >>>> @@ -24,6 +24,7 @@ typedef enum {
> > >>>>       */
> > >>>>      IF_NONE = 0,
> > >>>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD,
> > >>>> IF_VIRTIO, IF_XEN,
> > >>>> +    IF_EMMC,
> > >>>>      IF_COUNT
> > >>>>  } BlockInterfaceType;
> > >>>>
> > >>>> diff --git a/blockdev.c b/blockdev.c index cd438e6..390d43c 100644
> > >>>> --- a/blockdev.c
> > >>>> +++ b/blockdev.c
> > >>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> > >>>>      [IF_SD] = "sd",
> > >>>>      [IF_VIRTIO] = "virtio",
> > >>>>      [IF_XEN] = "xen",
> > >>>> +    [IF_EMMC] = "emmc",
> > >>>>  };
> > >>>
> > >>> We don't need to introduce support for the legacy -drive magic.
> > >>>
> > >>> -device should be enough for this device, right?
> > >>
> > >> External interface extensions need rationale: why do we want / need it?
> > >> The commit message neglects to provide one.
> > >>
> > >> Even more so when the interface in question is in a state like -drive
> > >> is.
> > >
> > > I wouldn't be too nasty about -drive;  for me I still find it the
> > > easiest way to start a VM.
> > 
> > But eMMC isn't a bus where you can plug drives, it is soldered on-board and is
> > mmio mapped to a fixed address. I don't see the point of having a drive
> > interface for it...
> [Sai Pavan Boddu] Yeah, this makes sense but having a drive would be a simple implementation without disturbing much in the sd card emulation code. And its just easy to use, just as how sd cards are inserted.
> 
> I need to see, how easy it would be with -device.


Lets see what your command line looks like for starting it with emmc.

Dave

> Thanks,
> Sai Pavan
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
RE: [PATCH v2 01/22] block: add eMMC block device type
Posted by Sai Pavan Boddu 4 years, 9 months ago 
Hi Philippe,

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> Sent: Monday, February 22, 2021 5:34 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent Palatin
> <vpalatin@chromium.org>; Dr. David Alan Gilbert <dgilbert@redhat.com>;
> Thomas Huth <thuth@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>;
> Peter Maydell <peter.maydell@linaro.org>; Alistair Francis
> <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc Michel
> <luc.michel@greensocs.com>; Paolo Bonzini <pbonzini@redhat.com>
> Cc: Sai Pavan Boddu <saipava@xilinx.com>; qemu-devel@nongnu.org; qemu-
> block@nongnu.org
> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> 
> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> > From: Vincent Palatin <vpalatin@chromium.org>
> >
> > Add new block device type.
> >
> > Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> > [SPB: Rebased over 5.1 version]
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  include/sysemu/blockdev.h | 1 +
> >  blockdev.c                | 1 +
> >  2 files changed, 2 insertions(+)
> >
> > diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> > index 3b5fcda..eefae9f 100644
> > --- a/include/sysemu/blockdev.h
> > +++ b/include/sysemu/blockdev.h
> > @@ -24,6 +24,7 @@ typedef enum {
> >       */
> >      IF_NONE = 0,
> >      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO,
> > IF_XEN,
> > +    IF_EMMC,
> >      IF_COUNT
> >  } BlockInterfaceType;
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index cd438e6..390d43c 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> >      [IF_SD] = "sd",
> >      [IF_VIRTIO] = "virtio",
> >      [IF_XEN] = "xen",
> > +    [IF_EMMC] = "emmc",
> >  };
> 
> We don't need to introduce support for the legacy -drive magic.
> 
> -device should be enough for this device, right?
[Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we anyway need blockdev support for this, which would require us the use -drive.

Can you give some pointers, how to approach this ?

Regards,
Sai Pavan
Re: [PATCH v2 01/22] block: add eMMC block device type
Posted by Stefan Hajnoczi 4 years, 9 months ago 
On Tue, Feb 23, 2021 at 05:35:20PM +0000, Sai Pavan Boddu wrote:
> Hi Philippe,
> 
> > -----Original Message-----
> > From: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Sent: Monday, February 22, 2021 5:34 PM
> > To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
> > <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> > <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> > <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> > <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent Palatin
> > <vpalatin@chromium.org>; Dr. David Alan Gilbert <dgilbert@redhat.com>;
> > Thomas Huth <thuth@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>;
> > Peter Maydell <peter.maydell@linaro.org>; Alistair Francis
> > <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc Michel
> > <luc.michel@greensocs.com>; Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Sai Pavan Boddu <saipava@xilinx.com>; qemu-devel@nongnu.org; qemu-
> > block@nongnu.org
> > Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> > 
> > On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> > > From: Vincent Palatin <vpalatin@chromium.org>
> > >
> > > Add new block device type.
> > >
> > > Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> > > [SPB: Rebased over 5.1 version]
> > > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > > ---
> > >  include/sysemu/blockdev.h | 1 +
> > >  blockdev.c                | 1 +
> > >  2 files changed, 2 insertions(+)
> > >
> > > diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> > > index 3b5fcda..eefae9f 100644
> > > --- a/include/sysemu/blockdev.h
> > > +++ b/include/sysemu/blockdev.h
> > > @@ -24,6 +24,7 @@ typedef enum {
> > >       */
> > >      IF_NONE = 0,
> > >      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO,
> > > IF_XEN,
> > > +    IF_EMMC,
> > >      IF_COUNT
> > >  } BlockInterfaceType;
> > >
> > > diff --git a/blockdev.c b/blockdev.c
> > > index cd438e6..390d43c 100644
> > > --- a/blockdev.c
> > > +++ b/blockdev.c
> > > @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> > >      [IF_SD] = "sd",
> > >      [IF_VIRTIO] = "virtio",
> > >      [IF_XEN] = "xen",
> > > +    [IF_EMMC] = "emmc",
> > >  };
> > 
> > We don't need to introduce support for the legacy -drive magic.
> > 
> > -device should be enough for this device, right?
> [Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we anyway need blockdev support for this, which would require us the use -drive.
> 
> Can you give some pointers, how to approach this ?

It is probably not necessary to add a new IF_ constant. Would this work:

  -drive if=none,id=emmc0,file=test.img,format=raw
  -device emmc,...,drive=emmc0

Or the more modern:

  -blockdev node-name=emmc0,driver=file,filename=test.img
  -device emmc,...,drive=emmc0

?

(The syntax might need small tweaks but is shows the general idea.)

Stefan
Re: [PATCH v2 01/22] block: add eMMC block device type
Posted by Cédric Le Goater 4 years, 9 months ago 
On 2/24/21 12:40 PM, Stefan Hajnoczi wrote:
> On Tue, Feb 23, 2021 at 05:35:20PM +0000, Sai Pavan Boddu wrote:
>> Hi Philippe,
>>
>>> -----Original Message-----
>>> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> Sent: Monday, February 22, 2021 5:34 PM
>>> To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
>>> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
>>> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
>>> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
>>> <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent Palatin
>>> <vpalatin@chromium.org>; Dr. David Alan Gilbert <dgilbert@redhat.com>;
>>> Thomas Huth <thuth@redhat.com>; Stefan Hajnoczi <stefanha@redhat.com>;
>>> Peter Maydell <peter.maydell@linaro.org>; Alistair Francis
>>> <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc Michel
>>> <luc.michel@greensocs.com>; Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Sai Pavan Boddu <saipava@xilinx.com>; qemu-devel@nongnu.org; qemu-
>>> block@nongnu.org
>>> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
>>>
>>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
>>>> From: Vincent Palatin <vpalatin@chromium.org>
>>>>
>>>> Add new block device type.
>>>>
>>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>>>> [SPB: Rebased over 5.1 version]
>>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>> ---
>>>>  include/sysemu/blockdev.h | 1 +
>>>>  blockdev.c                | 1 +
>>>>  2 files changed, 2 insertions(+)
>>>>
>>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
>>>> index 3b5fcda..eefae9f 100644
>>>> --- a/include/sysemu/blockdev.h
>>>> +++ b/include/sysemu/blockdev.h
>>>> @@ -24,6 +24,7 @@ typedef enum {
>>>>       */
>>>>      IF_NONE = 0,
>>>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO,
>>>> IF_XEN,
>>>> +    IF_EMMC,
>>>>      IF_COUNT
>>>>  } BlockInterfaceType;
>>>>
>>>> diff --git a/blockdev.c b/blockdev.c
>>>> index cd438e6..390d43c 100644
>>>> --- a/blockdev.c
>>>> +++ b/blockdev.c
>>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
>>>>      [IF_SD] = "sd",
>>>>      [IF_VIRTIO] = "virtio",
>>>>      [IF_XEN] = "xen",
>>>> +    [IF_EMMC] = "emmc",
>>>>  };
>>>
>>> We don't need to introduce support for the legacy -drive magic.
>>>
>>> -device should be enough for this device, right?
>> [Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we anyway need blockdev support for this, which would require us the use -drive.
>>
>> Can you give some pointers, how to approach this ?
> 
> It is probably not necessary to add a new IF_ constant. Would this work:
> 
>   -drive if=none,id=emmc0,file=test.img,format=raw
>   -device emmc,...,drive=emmc0
> 
> Or the more modern:
> 
>   -blockdev node-name=emmc0,driver=file,filename=test.img
>   -device emmc,...,drive=emmc0
> 
> ?
> 
> (The syntax might need small tweaks but is shows the general idea.)

Yes. This is better. 

We could have an "emmc" device inheriting from "sd-card". The "emmc" 
property would not be necessary anymore and may be, we could cleanup 
up some parts doing : 

    if (sd->emmc) { /* eMMC */
        ...
    } else {

    }

with SDCardClass handlers. the SWITCH_FUNCTION command is a good 
candidate, CMD8 also.

C.
RE: [PATCH v2 01/22] block: add eMMC block device type
Posted by Sai Pavan Boddu 4 years, 9 months ago 
Hi Cedric,


> -----Original Message-----
> From: Cédric Le Goater <clg@kaod.org>
> Sent: Wednesday, February 24, 2021 7:25 PM
> To: Stefan Hajnoczi <stefanha@redhat.com>; Sai Pavan Boddu
> <saipava@xilinx.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>; Markus Armbruster
> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> <joel@jms.id.au>; Vincent Palatin <vpalatin@chromium.org>; Dr. David Alan
> Gilbert <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Peter
> Maydell <peter.maydell@linaro.org>; Alistair Francis
> <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc Michel
> <luc.michel@greensocs.com>; Paolo Bonzini <pbonzini@redhat.com>; qemu-
> devel@nongnu.org; qemu-block@nongnu.org
> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> 
> On 2/24/21 12:40 PM, Stefan Hajnoczi wrote:
> > On Tue, Feb 23, 2021 at 05:35:20PM +0000, Sai Pavan Boddu wrote:
> >> Hi Philippe,
> >>
> >>> -----Original Message-----
> >>> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>> Sent: Monday, February 22, 2021 5:34 PM
> >>> To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
> >>> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> >>> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> >>> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel
> >>> Stanley <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent
> >>> Palatin <vpalatin@chromium.org>; Dr. David Alan Gilbert
> >>> <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Stefan
> >>> Hajnoczi <stefanha@redhat.com>; Peter Maydell
> >>> <peter.maydell@linaro.org>; Alistair Francis
> >>> <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc
> >>> Michel <luc.michel@greensocs.com>; Paolo Bonzini
> >>> <pbonzini@redhat.com>
> >>> Cc: Sai Pavan Boddu <saipava@xilinx.com>; qemu-devel@nongnu.org;
> >>> qemu- block@nongnu.org
> >>> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> >>>
> >>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> >>>> From: Vincent Palatin <vpalatin@chromium.org>
> >>>>
> >>>> Add new block device type.
> >>>>
> >>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> >>>> [SPB: Rebased over 5.1 version]
> >>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> >>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >>>> ---
> >>>>  include/sysemu/blockdev.h | 1 +
> >>>>  blockdev.c                | 1 +
> >>>>  2 files changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> >>>> index 3b5fcda..eefae9f 100644
> >>>> --- a/include/sysemu/blockdev.h
> >>>> +++ b/include/sysemu/blockdev.h
> >>>> @@ -24,6 +24,7 @@ typedef enum {
> >>>>       */
> >>>>      IF_NONE = 0,
> >>>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD,
> >>>> IF_VIRTIO, IF_XEN,
> >>>> +    IF_EMMC,
> >>>>      IF_COUNT
> >>>>  } BlockInterfaceType;
> >>>>
> >>>> diff --git a/blockdev.c b/blockdev.c index cd438e6..390d43c 100644
> >>>> --- a/blockdev.c
> >>>> +++ b/blockdev.c
> >>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> >>>>      [IF_SD] = "sd",
> >>>>      [IF_VIRTIO] = "virtio",
> >>>>      [IF_XEN] = "xen",
> >>>> +    [IF_EMMC] = "emmc",
> >>>>  };
> >>>
> >>> We don't need to introduce support for the legacy -drive magic.
> >>>
> >>> -device should be enough for this device, right?
> >> [Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we
> anyway need blockdev support for this, which would require us the use -drive.
> >>
> >> Can you give some pointers, how to approach this ?
> >
> > It is probably not necessary to add a new IF_ constant. Would this work:
> >
> >   -drive if=none,id=emmc0,file=test.img,format=raw
> >   -device emmc,...,drive=emmc0
> >
> > Or the more modern:
> >
> >   -blockdev node-name=emmc0,driver=file,filename=test.img
> >   -device emmc,...,drive=emmc0
> >
> > ?
> >
> > (The syntax might need small tweaks but is shows the general idea.)
> 
> Yes. This is better.
> 
> We could have an "emmc" device inheriting from "sd-card". The "emmc"
> property would not be necessary anymore and may be, we could cleanup up
> some parts doing :
> 
>     if (sd->emmc) { /* eMMC */
>         ...
>     } else {
> 
>     }
> 
> with SDCardClass handlers. the SWITCH_FUNCTION command is a good
> candidate, CMD8 also.
[Sai Pavan Boddu] Nice, this approach looks clean.
But we still may be depending on emmc property. Not sure!

I would get back with v3, your review on those patches would be great.

Thanks & Regards,
Sai Pavan
> 
> C.
Re: [PATCH v2 01/22] block: add eMMC block device type
Posted by Cédric Le Goater 4 years, 9 months ago 
On 2/24/21 8:13 PM, Sai Pavan Boddu wrote:
> Hi Cedric,
> 
> 
>> -----Original Message-----
>> From: Cédric Le Goater <clg@kaod.org>
>> Sent: Wednesday, February 24, 2021 7:25 PM
>> To: Stefan Hajnoczi <stefanha@redhat.com>; Sai Pavan Boddu
>> <saipava@xilinx.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>; Markus Armbruster
>> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
>> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
>> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
>> <joel@jms.id.au>; Vincent Palatin <vpalatin@chromium.org>; Dr. David Alan
>> Gilbert <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Peter
>> Maydell <peter.maydell@linaro.org>; Alistair Francis
>> <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc Michel
>> <luc.michel@greensocs.com>; Paolo Bonzini <pbonzini@redhat.com>; qemu-
>> devel@nongnu.org; qemu-block@nongnu.org
>> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
>>
>> On 2/24/21 12:40 PM, Stefan Hajnoczi wrote:
>>> On Tue, Feb 23, 2021 at 05:35:20PM +0000, Sai Pavan Boddu wrote:
>>>> Hi Philippe,
>>>>
>>>>> -----Original Message-----
>>>>> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>> Sent: Monday, February 22, 2021 5:34 PM
>>>>> To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
>>>>> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
>>>>> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
>>>>> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel
>>>>> Stanley <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent
>>>>> Palatin <vpalatin@chromium.org>; Dr. David Alan Gilbert
>>>>> <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Stefan
>>>>> Hajnoczi <stefanha@redhat.com>; Peter Maydell
>>>>> <peter.maydell@linaro.org>; Alistair Francis
>>>>> <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc
>>>>> Michel <luc.michel@greensocs.com>; Paolo Bonzini
>>>>> <pbonzini@redhat.com>
>>>>> Cc: Sai Pavan Boddu <saipava@xilinx.com>; qemu-devel@nongnu.org;
>>>>> qemu- block@nongnu.org
>>>>> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
>>>>>
>>>>> On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
>>>>>> From: Vincent Palatin <vpalatin@chromium.org>
>>>>>>
>>>>>> Add new block device type.
>>>>>>
>>>>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>>>>>> [SPB: Rebased over 5.1 version]
>>>>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>>>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>>>> ---
>>>>>>  include/sysemu/blockdev.h | 1 +
>>>>>>  blockdev.c                | 1 +
>>>>>>  2 files changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
>>>>>> index 3b5fcda..eefae9f 100644
>>>>>> --- a/include/sysemu/blockdev.h
>>>>>> +++ b/include/sysemu/blockdev.h
>>>>>> @@ -24,6 +24,7 @@ typedef enum {
>>>>>>       */
>>>>>>      IF_NONE = 0,
>>>>>>      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD,
>>>>>> IF_VIRTIO, IF_XEN,
>>>>>> +    IF_EMMC,
>>>>>>      IF_COUNT
>>>>>>  } BlockInterfaceType;
>>>>>>
>>>>>> diff --git a/blockdev.c b/blockdev.c index cd438e6..390d43c 100644
>>>>>> --- a/blockdev.c
>>>>>> +++ b/blockdev.c
>>>>>> @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
>>>>>>      [IF_SD] = "sd",
>>>>>>      [IF_VIRTIO] = "virtio",
>>>>>>      [IF_XEN] = "xen",
>>>>>> +    [IF_EMMC] = "emmc",
>>>>>>  };
>>>>>
>>>>> We don't need to introduce support for the legacy -drive magic.
>>>>>
>>>>> -device should be enough for this device, right?
>>>> [Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we
>> anyway need blockdev support for this, which would require us the use -drive.
>>>>
>>>> Can you give some pointers, how to approach this ?
>>>
>>> It is probably not necessary to add a new IF_ constant. Would this work:
>>>
>>>   -drive if=none,id=emmc0,file=test.img,format=raw
>>>   -device emmc,...,drive=emmc0
>>>
>>> Or the more modern:
>>>
>>>   -blockdev node-name=emmc0,driver=file,filename=test.img
>>>   -device emmc,...,drive=emmc0
>>>
>>> ?
>>>
>>> (The syntax might need small tweaks but is shows the general idea.)
>>
>> Yes. This is better.
>>
>> We could have an "emmc" device inheriting from "sd-card". The "emmc"
>> property would not be necessary anymore and may be, we could cleanup up
>> some parts doing :
>>
>>     if (sd->emmc) { /* eMMC */
>>         ...
>>     } else {
>>
>>     }
>>
>> with SDCardClass handlers. the SWITCH_FUNCTION command is a good
>> candidate, CMD8 also.
> [Sai Pavan Boddu] Nice, this approach looks clean.
> But we still may be depending on emmc property. Not sure!

It could be a SDCardClass attribute instead since it's a constant for
the emmc device.

C. 

> 
> I would get back with v3, your review on those patches would be great.
> 
> Thanks & Regards,
> Sai Pavan
>>
>> C.
RE: [PATCH v2 01/22] block: add eMMC block device type
Posted by Sai Pavan Boddu 4 years, 9 months ago 
Hi Stefan

> -----Original Message-----
> From: Stefan Hajnoczi <stefanha@redhat.com>
> Sent: Wednesday, February 24, 2021 5:10 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>; Markus Armbruster
> <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel Stanley
> <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent Palatin
> <vpalatin@chromium.org>; Dr. David Alan Gilbert <dgilbert@redhat.com>;
> Thomas Huth <thuth@redhat.com>; Peter Maydell
> <peter.maydell@linaro.org>; Alistair Francis <alistair.francis@wdc.com>; Edgar
> Iglesias <edgari@xilinx.com>; Luc Michel <luc.michel@greensocs.com>; Paolo
> Bonzini <pbonzini@redhat.com>; qemu-devel@nongnu.org; qemu-
> block@nongnu.org
> Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> 
> On Tue, Feb 23, 2021 at 05:35:20PM +0000, Sai Pavan Boddu wrote:
> > Hi Philippe,
> >
> > > -----Original Message-----
> > > From: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > Sent: Monday, February 22, 2021 5:34 PM
> > > To: Sai Pavan Boddu <saipava@xilinx.com>; Markus Armbruster
> > > <armbru@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> > > <mreitz@redhat.com>; Vladimir Sementsov-Ogievskiy
> > > <vsementsov@virtuozzo.com>; Eric Blake <eblake@redhat.com>; Joel
> > > Stanley <joel@jms.id.au>; Cédric Le Goater <clg@kaod.org>; Vincent
> > > Palatin <vpalatin@chromium.org>; Dr. David Alan Gilbert
> > > <dgilbert@redhat.com>; Thomas Huth <thuth@redhat.com>; Stefan
> > > Hajnoczi <stefanha@redhat.com>; Peter Maydell
> > > <peter.maydell@linaro.org>; Alistair Francis
> > > <alistair.francis@wdc.com>; Edgar Iglesias <edgari@xilinx.com>; Luc
> > > Michel <luc.michel@greensocs.com>; Paolo Bonzini
> > > <pbonzini@redhat.com>
> > > Cc: Sai Pavan Boddu <saipava@xilinx.com>; qemu-devel@nongnu.org;
> > > qemu- block@nongnu.org
> > > Subject: Re: [PATCH v2 01/22] block: add eMMC block device type
> > >
> > > On 2/22/21 9:20 AM, Sai Pavan Boddu wrote:
> > > > From: Vincent Palatin <vpalatin@chromium.org>
> > > >
> > > > Add new block device type.
> > > >
> > > > Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> > > > [SPB: Rebased over 5.1 version]
> > > > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> > > > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > > > ---
> > > >  include/sysemu/blockdev.h | 1 +
> > > >  blockdev.c                | 1 +
> > > >  2 files changed, 2 insertions(+)
> > > >
> > > > diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
> > > > index 3b5fcda..eefae9f 100644
> > > > --- a/include/sysemu/blockdev.h
> > > > +++ b/include/sysemu/blockdev.h
> > > > @@ -24,6 +24,7 @@ typedef enum {
> > > >       */
> > > >      IF_NONE = 0,
> > > >      IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD,
> > > > IF_VIRTIO, IF_XEN,
> > > > +    IF_EMMC,
> > > >      IF_COUNT
> > > >  } BlockInterfaceType;
> > > >
> > > > diff --git a/blockdev.c b/blockdev.c index cd438e6..390d43c 100644
> > > > --- a/blockdev.c
> > > > +++ b/blockdev.c
> > > > @@ -83,6 +83,7 @@ static const char *const if_name[IF_COUNT] = {
> > > >      [IF_SD] = "sd",
> > > >      [IF_VIRTIO] = "virtio",
> > > >      [IF_XEN] = "xen",
> > > > +    [IF_EMMC] = "emmc",
> > > >  };
> > >
> > > We don't need to introduce support for the legacy -drive magic.
> > >
> > > -device should be enough for this device, right?
> > [Sai Pavan Boddu] I was seeing to use -device for emmc. But I see we anyway
> need blockdev support for this, which would require us the use -drive.
> >
> > Can you give some pointers, how to approach this ?
> 
> It is probably not necessary to add a new IF_ constant. Would this work:
> 
>   -drive if=none,id=emmc0,file=test.img,format=raw
>   -device emmc,...,drive=emmc0
[Sai Pavan Boddu] Great, this works for me.
> 
> Or the more modern:
> 
>   -blockdev node-name=emmc0,driver=file,filename=test.img
>   -device emmc,...,drive=emmc0
[Sai Pavan Boddu] Thanks, I would try to follow the modern approach then!

Regards,
Sai Pavan
> 
> ?
> 
> (The syntax might need small tweaks but is shows the general idea.)
> 
> Stefan

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.