RE: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces

2020-06-30 Thread Dmitry Fomichev
> -Original Message- > From: Klaus Jensen > Sent: Tuesday, June 30, 2020 4:30 PM > To: Niklas Cassel > Cc: qemu-block@nongnu.org; Klaus Jensen ; > qemu-de...@nongnu.org; Keith Busch ; Max Reitz > ; Kevin Wolf ; Javier Gonzalez > ; Maxim Levitsky ; > Philippe Mathieu-Daudé ; Dmitry

Re: [PATCH v2 12/18] hw/block/nvme: Simulate Zone Active excursions

2020-06-30 Thread Alistair Francis
On Wed, Jun 17, 2020 at 2:52 PM Dmitry Fomichev wrote: > > Added a Boolean flag to turn on simulation of Zone Active Excursions. > If the flag, "active_excursions", is set to true, the driver will try > to finish one of the currently open zone if max active zones limit is > going to get exceeded.

Re: [PATCH v2 11/18] hw/block/nvme: Introduce max active and open zone limits

2020-06-30 Thread Alistair Francis
On Wed, Jun 17, 2020 at 3:07 PM Dmitry Fomichev wrote: > > Added two module properties, "max_active" and "max_open" to control > the maximum number of zones that can be active or open. Once these > variables are set to non-default values, the driver checks these > limits during I/O and returns

Re: [PATCH v2 08/18] hw/block/nvme: Make Zoned NS Command Set definitions

2020-06-30 Thread Alistair Francis
On Wed, Jun 17, 2020 at 2:51 PM Dmitry Fomichev wrote: > > Define values and structures that are needed to support Zoned > Namespace Command Set (NVMe TP 4053) in PCI NVMe controller emulator. > > All new protocol definitions are located in include/block/nvme.h > and everything added that is

Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces

2020-06-30 Thread Klaus Jensen
On Jun 30 08:42, Keith Busch wrote: > On Tue, Jun 30, 2020 at 04:09:46PM +0200, Philippe Mathieu-Daudé wrote: > > What I see doable for the following days is: > > - hw/block/nvme: Fix I/O BAR structure [3] > > - hw/block/nvme: handle transient dma errors > > - hw/block/nvme: bump to v1.3 > > >

Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces

2020-06-30 Thread Klaus Jensen
On Jun 30 12:59, Niklas Cassel wrote: > On Tue, Jun 30, 2020 at 12:01:29PM +0200, Klaus Jensen wrote: > > From: Klaus Jensen > > > > Hi all, > > Hello Klaus, > Hi Niklas, > > > > * the controller uses timers to autonomously finish zones (wrt. FRL) > > AFAICT, Dmitry's patches does this

Re: [PATCH v2 06/18] hw/block/nvme: Define trace events related to NS Types

2020-06-30 Thread Alistair Francis
On Wed, Jun 17, 2020 at 2:46 PM Dmitry Fomichev wrote: > > A few trace events are defined that are relevant to implementing > Namespace Types (NVMe TP 4056). > > Signed-off-by: Dmitry Fomichev Reviewed-by: Alistair Francis Alistair > --- > hw/block/trace-events | 11 +++ > 1 file

[PATCH v2 11/12] block/nvme: Simplify nvme_create_queue_pair() arguments

2020-06-30 Thread Philippe Mathieu-Daudé
nvme_create_queue_pair() doesn't require BlockDriverState anymore. Replace it by BDRVNVMeState and AioContext to simplify. Reviewed-by: Stefan Hajnoczi Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git

[PATCH v2 07/12] block/nvme: Replace qemu_try_blockalign0 by qemu_try_blockalign/memset

2020-06-30 Thread Philippe Mathieu-Daudé
In the next commit we'll get rid of qemu_try_blockalign(). To ease review, first replace qemu_try_blockalign0() by explicit calls to qemu_try_blockalign() and memset(). Reviewed-by: Stefan Hajnoczi Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 16 +--- 1 file changed, 9

[PATCH v2 09/12] block/nvme: Simplify nvme_init_queue() arguments

2020-06-30 Thread Philippe Mathieu-Daudé
nvme_init_queue() doesn't require BlockDriverState anymore. Replace it by BDRVNVMeState to simplify. Reviewed-by: Stefan Hajnoczi Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index

[PATCH v2 10/12] block/nvme: Replace BDRV_POLL_WHILE by AIO_WAIT_WHILE

2020-06-30 Thread Philippe Mathieu-Daudé
BDRV_POLL_WHILE() is defined as: #define BDRV_POLL_WHILE(bs, cond) ({ \ BlockDriverState *bs_ = (bs); \ AIO_WAIT_WHILE(bdrv_get_aio_context(bs_), \ cond); }) As we will remove the BlockDriverState use in the next commit, start by using the

[PATCH v2 12/12] block/nvme: Use per-queue AIO context

2020-06-30 Thread Philippe Mathieu-Daudé
To be able to use multiple queues on the same hardware, we need to have each queue able to receive IRQ notifications in the correct AIO context. The AIO context and the notification handler have to be proper to each queue, not to the block driver. Move aio_context and irq_notifier from

[PATCH v2 05/12] block/nvme: Rename local variable

2020-06-30 Thread Philippe Mathieu-Daudé
We are going to modify the code in the next commit. Renaming the 'resp' variable to 'id' first makes the next commit easier to review. No logical changes. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff

[PATCH v2 06/12] block/nvme: Use union of NvmeIdCtrl / NvmeIdNs structures

2020-06-30 Thread Philippe Mathieu-Daudé
We allocate an unique chunk of memory then use it for two different structures. By using an union, we make it clear the data is overlapping (and we can remove the casts). Suggested-by: Stefan Hajnoczi Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 31 +++

[PATCH v2 08/12] block/nvme: Replace qemu_try_blockalign(bs) by qemu_try_memalign(pg_sz)

2020-06-30 Thread Philippe Mathieu-Daudé
qemu_try_blockalign() is a generic API that call back to the block driver to return its page alignment. As we call from within the very same driver, we already know to page alignment stored in our state. Remove indirections and use the value from BDRVNVMeState. This change is required to later

[PATCH v2 03/12] block/nvme: Let nvme_create_queue_pair() fail gracefully

2020-06-30 Thread Philippe Mathieu-Daudé
As nvme_create_queue_pair() is allowed to fail, replace the alloc() calls by try_alloc() to avoid aborting QEMU. Reviewed-by: Stefan Hajnoczi Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/block/nvme.c

[PATCH v2 04/12] block/nvme: Define QUEUE_INDEX macros to ease code review

2020-06-30 Thread Philippe Mathieu-Daudé
Use definitions instead of '0' or '1' indexes. Also this will be useful when using multi-queues later. Reviewed-by: Stefan Hajnoczi Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 33 +++-- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git

[PATCH v2 00/12] block/nvme: Various cleanups required to use multiple queues

2020-06-30 Thread Philippe Mathieu-Daudé
Hi, This series is mostly code rearrangement (cleanups) to be able to split the hardware code from the block driver code, to be able to use multiple queues on the same hardware, or multiple block drivers on the same hardware. Missing review: 1, 2, 5, 6, 8, 12. Since v1: - rebased - use SCALE_MS

[PATCH v2 02/12] block/nvme: Avoid further processing if trace event not enabled

2020-06-30 Thread Philippe Mathieu-Daudé
Avoid further processing if TRACE_NVME_SUBMIT_COMMAND_RAW is not enabled. This is an untested intend of performance optimization. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/nvme.c b/block/nvme.c index

[PATCH v2 01/12] block/nvme: Replace magic value by SCALE_MS definition

2020-06-30 Thread Philippe Mathieu-Daudé
Use self-explicit SCALE_MS definition instead of magic value. Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/nvme.c b/block/nvme.c index 374e268915..2f5e3c2adf 100644 --- a/block/nvme.c +++ b/block/nvme.c @@

Re: [PATCH v2 05/18] hw/block/nvme: Introduce the Namespace Types definitions

2020-06-30 Thread Keith Busch
On Tue, Jun 30, 2020 at 10:02:15AM +, Niklas Cassel wrote: > On Mon, Jun 29, 2020 at 07:12:47PM -0700, Alistair Francis wrote: > > On Wed, Jun 17, 2020 at 2:47 PM Dmitry Fomichev > > wrote: > > > +uint16_tctrlid; > > > > Shouldn't this be CNTID? > > From the NVMe spec: >

Re: [PATCH v3 3/4] hw/block/nvme: Fix pmrmsc register size

2020-06-30 Thread Klaus Jensen
On Jun 30 17:16, Philippe Mathieu-Daudé wrote: > On 6/30/20 5:10 PM, Andrzej Jakowski wrote: > > On 6/30/20 4:04 AM, Philippe Mathieu-Daudé wrote: > >> The Persistent Memory Region Controller Memory Space Control > >> register is 64-bit wide. See 'Figure 68: Register Definition' > >> of the 'NVM

Re: [PATCH v2 05/18] hw/block/nvme: Introduce the Namespace Types definitions

2020-06-30 Thread Niklas Cassel
On Tue, Jun 30, 2020 at 06:57:16AM +0200, Klaus Jensen wrote: > On Jun 18 06:34, Dmitry Fomichev wrote: > > From: Niklas Cassel > > > > Define the structures and constants required to implement > > Namespace Types support. > > > > Signed-off-by: Niklas Cassel > > Signed-off-by: Dmitry Fomichev

Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces

2020-06-30 Thread Keith Busch
On Tue, Jun 30, 2020 at 04:09:46PM +0200, Philippe Mathieu-Daudé wrote: > What I see doable for the following days is: > - hw/block/nvme: Fix I/O BAR structure [3] > - hw/block/nvme: handle transient dma errors > - hw/block/nvme: bump to v1.3 These look like sensible patches to rebase future

Re: [PATCH v3 3/4] hw/block/nvme: Fix pmrmsc register size

2020-06-30 Thread Philippe Mathieu-Daudé
On 6/30/20 5:10 PM, Andrzej Jakowski wrote: > On 6/30/20 4:04 AM, Philippe Mathieu-Daudé wrote: >> The Persistent Memory Region Controller Memory Space Control >> register is 64-bit wide. See 'Figure 68: Register Definition' >> of the 'NVM Express Base Specification Revision 1.4'. >> >> Fixes:

Re: [PATCH v3 3/4] hw/block/nvme: Fix pmrmsc register size

2020-06-30 Thread Andrzej Jakowski
On 6/30/20 4:04 AM, Philippe Mathieu-Daudé wrote: > The Persistent Memory Region Controller Memory Space Control > register is 64-bit wide. See 'Figure 68: Register Definition' > of the 'NVM Express Base Specification Revision 1.4'. > > Fixes: 6cf9413229 ("introduce PMR support from NVMe 1.4

Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces

2020-06-30 Thread Niklas Cassel
On Tue, Jun 30, 2020 at 12:01:29PM +0200, Klaus Jensen wrote: > From: Klaus Jensen > > Hi all, Hello Klaus, > > This series adds support for TP 4056 ("Namespace Types") and TP 4053 > ("Zoned Namespaces") and is an alternative implementation to the one > submitted by Dmitry[1]. > > While I

Re: [PATCH v2 05/18] hw/block/nvme: Introduce the Namespace Types definitions

2020-06-30 Thread Niklas Cassel
On Mon, Jun 29, 2020 at 07:12:47PM -0700, Alistair Francis wrote: > On Wed, Jun 17, 2020 at 2:47 PM Dmitry Fomichev > wrote: > > > > From: Niklas Cassel > > > > Define the structures and constants required to implement > > Namespace Types support. > > > > Signed-off-by: Niklas Cassel > >

Re: [PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces

2020-06-30 Thread Philippe Mathieu-Daudé
On 6/30/20 2:59 PM, Niklas Cassel wrote: > On Tue, Jun 30, 2020 at 12:01:29PM +0200, Klaus Jensen wrote: >> From: Klaus Jensen >> >> Hi all, > > Hello Klaus, > >> >> This series adds support for TP 4056 ("Namespace Types") and TP 4053 >> ("Zoned Namespaces") and is an alternative implementation

[PATCH v7 15/17] hw/sd/sdcard: Correctly display the command name in trace events

2020-06-30 Thread Philippe Mathieu-Daudé
Some ACMD were incorrectly displayed. Fix by remembering if we are processing a ACMD (with current_cmd_is_acmd) and add the sd_current_cmd_name() helper, which display to correct name regardless it is a CMD or ACMD. Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 17 ++--- 1

[PATCH v7 16/17] hw/sd/sdcard: Display offset in read/write_data() trace events

2020-06-30 Thread Philippe Mathieu-Daudé
Having 'base address' and 'relative offset' displayed separately is more helpful than the absolute address. Reviewed-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 8 hw/sd/trace-events | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff

[PATCH v7 14/17] hw/sd/sdcard: Make iolen unsigned

2020-06-30 Thread Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé I/O request length can not be negative. Signed-off-by: Philippe Mathieu-Daudé --- v4: Use uint32_t (pm215) --- hw/sd/sd.c | 2 +- hw/sd/trace-events | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index

[PATCH v7 12/17] hw/sd/sdcard: Simplify cmd_valid_while_locked()

2020-06-30 Thread Philippe Mathieu-Daudé
cmd_valid_while_locked() only needs to read SDRequest->cmd, pass it directly and make it const. Reviewed-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index

[PATCH v7 07/17] hw/sd/sdcard: Move sd->size initialization

2020-06-30 Thread Philippe Mathieu-Daudé
Move sd->size initialization earlier to make the following patches easier to review. Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 871c30a67f..078b0e81ee 100644 --- a/hw/sd/sd.c +++

[PATCH v7 17/17] hw/sd/sdcard: Simplify realize() a bit

2020-06-30 Thread Philippe Mathieu-Daudé
We don't need to check if sd->blk is set twice. Reviewed-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 304fa4143a..8ef6715665 100644 --- a/hw/sd/sd.c +++

[PATCH v7 13/17] hw/sd/sdcard: Constify sd_crc*()'s message argument

2020-06-30 Thread Philippe Mathieu-Daudé
CRC functions don't modify the buffer argument, make it const. Reviewed-by: Alistair Francis Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 2946fe3040..364a6d1fcd 100644 ---

[PATCH v7 06/17] hw/sd/sdcard: Restrict Class 6 commands to SCSD cards

2020-06-30 Thread Philippe Mathieu-Daudé
Only SCSD cards support Class 6 (Block Oriented Write Protection) commands. "SD Specifications Part 1 Physical Layer Simplified Spec. v3.01" 4.3.14 Command Functional Difference in Card Capacity Types * Write Protected Group SDHC and SDXC do not support write-protected groups. Issuing

[PATCH v7 05/17] hw/sd/sdcard: Do not switch to ReceivingData if address is invalid

2020-06-30 Thread Philippe Mathieu-Daudé
Only move the state machine to ReceivingData if there is no pending error. This avoids later OOB access while processing commands queued. "SD Specifications Part 1 Physical Layer Simplified Spec. v3.01" 4.3.3 Data Read Read command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR

[PATCH v7 08/17] hw/sd/sdcard: Call sd_addr_to_wpnum where it is used, consider zero size

2020-06-30 Thread Philippe Mathieu-Daudé
Avoid setting 'sect' variable just once (its name is confuse anyway). Directly set 'sd->wpgrps_size'. Special case when size is zero. Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index

[PATCH v7 10/17] hw/sd/sdcard: Check address is in range

2020-06-30 Thread Philippe Mathieu-Daudé
As a defense, assert if the requested address is out of the card area. Suggested-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 548745614e..5d1b314a32

[PATCH v7 11/17] hw/sd/sdcard: Update the SDState documentation

2020-06-30 Thread Philippe Mathieu-Daudé
Add more descriptive comments to keep a clear separation between static property vs runtime changeable. Suggested-by: Peter Maydell Reviewed-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git

[PATCH v7 03/17] hw/sd/sdcard: Move some definitions to use them earlier

2020-06-30 Thread Philippe Mathieu-Daudé
Move some definitions to use them earlier. Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index cac8d7d828..4816b4a462 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -80,6 +80,12 @@ enum

[PATCH v7 09/17] hw/sd/sdcard: Special case the -ENOMEDIUM error

2020-06-30 Thread Philippe Mathieu-Daudé
As we have no interest in the underlying block geometry, directly call blk_getlength(). We have to care about machines creating SD card with not drive attached (probably incorrect API use). Simply emit a warning when such Frankenstein cards of zero size are reset. Signed-off-by: Philippe

[PATCH v7 04/17] hw/sd/sdcard: Use the HWBLOCK_SIZE definition

2020-06-30 Thread Philippe Mathieu-Daudé
Replace the following different uses of the same value by the same HWBLOCK_SIZE definition: - 512 (magic value) - 0x200 (magic value) - 1 << HWBLOCK_SHIFT Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff

[PATCH v7 02/17] hw/sd/sdcard: Update coding style to make checkpatch.pl happy

2020-06-30 Thread Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé To make the next commit easier to review, clean this code first. Reviewed-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c

[PATCH v7 01/17] MAINTAINERS: Cc qemu-block mailing list

2020-06-30 Thread Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé We forgot to include the qemu-block mailing list while adding this section in commit 076a0fc32a7. Fix this. Suggested-by: Paolo Bonzini Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS

[PATCH v7 00/17] hw/sd/sdcard: Fix CVE-2020-13253 & cleanups

2020-06-30 Thread Philippe Mathieu-Daudé
Patches 5 & 6 fix CVE-2020-13253. The rest are (accumulated) cleanups. Since v6: Handle -ENOMEDIUM error Since v5: Fix incorrect use of sd_addr_to_wpnum() in sd_reset() Missing review: [PATCH 01/15] MAINTAINERS: Cc qemu-block mailing list [PATCH 03/15] hw/sd/sdcard: Move some definitions to use

Re: [PATCH v2 10/18] hw/block/nvme: Support Zoned Namespace Command Set

2020-06-30 Thread Klaus Jensen
On Jun 18 06:34, Dmitry Fomichev wrote: > The driver has been changed to advertise NVM Command Set when "zoned" > driver property is not set (default) and Zoned Namespace Command Set > otherwise. > > Handlers for three new NVMe commands introduced in Zoned Namespace > Command Set specification

Re: [PATCH v2 09/18] hw/block/nvme: Define Zoned NS Command Set trace events

2020-06-30 Thread Klaus Jensen
On Jun 18 06:34, Dmitry Fomichev wrote: > The Zoned Namespace Command Set / Namespace Types implementation that > is being introduced in this series adds a good number of trace events. > Combine all tracepoint definitions into a separate patch to make > reviewing more convenient. > >

Re: [PATCH v2 08/18] hw/block/nvme: Make Zoned NS Command Set definitions

2020-06-30 Thread Klaus Jensen
On Jun 30 13:44, Klaus Jensen wrote: > On Jun 18 06:34, Dmitry Fomichev wrote: > > Define values and structures that are needed to support Zoned > > Namespace Command Set (NVMe TP 4053) in PCI NVMe controller emulator. > > > > All new protocol definitions are located in include/block/nvme.h > >

Re: [PATCH v2 08/18] hw/block/nvme: Make Zoned NS Command Set definitions

2020-06-30 Thread Klaus Jensen
On Jun 18 06:34, Dmitry Fomichev wrote: > Define values and structures that are needed to support Zoned > Namespace Command Set (NVMe TP 4053) in PCI NVMe controller emulator. > > All new protocol definitions are located in include/block/nvme.h > and everything added that is specific to this

Re: [PATCH v2 07/18] hw/block/nvme: Add support for Namespace Types

2020-06-30 Thread Klaus Jensen
On Jun 18 06:34, Dmitry Fomichev wrote: > From: Niklas Cassel > > Namespace Types introduce a new command set, "I/O Command Sets", > that allows the host to retrieve the command sets associated with > a namespace. Introduce support for the command set, and enable > detection for the NVM Command

[PATCH v3 3/4] hw/block/nvme: Fix pmrmsc register size

2020-06-30 Thread Philippe Mathieu-Daudé
The Persistent Memory Region Controller Memory Space Control register is 64-bit wide. See 'Figure 68: Register Definition' of the 'NVM Express Base Specification Revision 1.4'. Fixes: 6cf9413229 ("introduce PMR support from NVMe 1.4 spec") Reported-by: Klaus Jensen Reviewed-by: Klaus Jensen

[PATCH v3 0/4] hw/block/nvme: Fix I/O BAR structure

2020-06-30 Thread Philippe Mathieu-Daudé
Improvements for the I/O BAR structure: - correct pmrmsc register size (Klaus) - pack structures - align to 4KB Since v2: - Added Klaus' tags with correct address $ git backport-diff -u v2 Key: [] : patches are identical [] : number of functional differences between upstream/downstream

[PATCH v3 2/4] hw/block/nvme: Use QEMU_PACKED on hardware/packet structures

2020-06-30 Thread Philippe Mathieu-Daudé
These structures either describe hardware registers, or commands ('packets') to send to the hardware. To forbid the compiler to optimize and change fields alignment, mark the structures as packed. Reviewed-by: Klaus Jensen Signed-off-by: Philippe Mathieu-Daudé --- include/block/nvme.h | 38

[PATCH v3 4/4] hw/block/nvme: Align I/O BAR to 4 KiB

2020-06-30 Thread Philippe Mathieu-Daudé
Simplify the NVMe emulated device by aligning the I/O BAR to 4 KiB. Reviewed-by: Klaus Jensen Signed-off-by: Philippe Mathieu-Daudé --- include/block/nvme.h | 2 ++ hw/block/nvme.c | 5 ++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/include/block/nvme.h

[PATCH v3 1/4] hw/block/nvme: Update specification URL

2020-06-30 Thread Philippe Mathieu-Daudé
At some point the URL changed, update it to avoid other developers to search for it. Reviewed-by: Klaus Jensen Signed-off-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index

Re: [PATCH v2 2/4] hw/block/nvme: Use QEMU_PACKED on hardware/packet structures

2020-06-30 Thread Klaus Jensen
On Jun 30 12:37, Philippe Mathieu-Daudé wrote: > These structures either describe hardware registers, or > commands ('packets') to send to the hardware. To forbid > the compiler to optimize and change fields alignment, > mark the structures as packed. > > Signed-off-by: Philippe Mathieu-Daudé

Re: [PATCH 2/4] migration: Add block-bitmap-mapping parameter

2020-06-30 Thread Dr. David Alan Gilbert
* Max Reitz (mre...@redhat.com) wrote: > This migration parameter allows mapping block node names and bitmap > names to aliases for the purpose of block dirty bitmap migration. > > This way, management tools can use different node and bitmap names on > the source and destination and pass the

Re: [PATCH v2 3/4] hw/block/nvme: Fix pmrmsc register size

2020-06-30 Thread Klaus Jensen
On Jun 30 12:37, Philippe Mathieu-Daudé wrote: > The Persistent Memory Region Controller Memory Space Control > register is 64-bit wide. See 'Figure 68: Register Definition' > of the 'NVM Express Base Specification Revision 1.4'. > > Fixes: 6cf9413229 ("introduce PMR support from NVMe 1.4 spec")

Re: [PATCH v2 1/4] hw/block/nvme: Update specification URL

2020-06-30 Thread Klaus Jensen
On Jun 30 12:37, Philippe Mathieu-Daudé wrote: > At some point the URL changed, update it to avoid other > developers to search for it. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Klaus Jensen > --- > hw/block/nvme.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >

Re: [PATCH v6 00/15] hw/sd/sdcard: Fix CVE-2020-13253 & cleanups

2020-06-30 Thread no-reply
Patchew URL: https://patchew.org/QEMU/20200630100342.27625-1-f4...@amsat.org/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN ===

Re: [PATCH v9 05/34] qcow2: Process QCOW2_CLUSTER_ZERO_ALLOC clusters in handle_copied()

2020-06-30 Thread Max Reitz
On 28.06.20 13:02, Alberto Garcia wrote: > When writing to a qcow2 file there are two functions that take a > virtual offset and return a host offset, possibly allocating new > clusters if necessary: > >- handle_copied() looks for normal data clusters that are already > allocated and

[PATCH v2 4/4] hw/block/nvme: Align I/O BAR to 4 KiB

2020-06-30 Thread Philippe Mathieu-Daudé
Simplify the NVMe emulated device by aligning the I/O BAR to 4 KiB. Reviewed-by: Klaus Jensen Signed-off-by: Philippe Mathieu-Daudé --- v2: Do not include 'cmd_set_specfic' (Klaus) --- include/block/nvme.h | 2 ++ hw/block/nvme.c | 5 ++--- 2 files changed, 4 insertions(+), 3 deletions(-)

[PATCH v2 1/4] hw/block/nvme: Update specification URL

2020-06-30 Thread Philippe Mathieu-Daudé
At some point the URL changed, update it to avoid other developers to search for it. Signed-off-by: Philippe Mathieu-Daudé --- hw/block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 1aee042d4c..6628d0a4ba 100644 ---

[PATCH v2 3/4] hw/block/nvme: Fix pmrmsc register size

2020-06-30 Thread Philippe Mathieu-Daudé
The Persistent Memory Region Controller Memory Space Control register is 64-bit wide. See 'Figure 68: Register Definition' of the 'NVM Express Base Specification Revision 1.4'. Fixes: 6cf9413229 ("introduce PMR support from NVMe 1.4 spec") Reported-by: Klaus Jensen Signed-off-by: Philippe

[PATCH v2 2/4] hw/block/nvme: Use QEMU_PACKED on hardware/packet structures

2020-06-30 Thread Philippe Mathieu-Daudé
These structures either describe hardware registers, or commands ('packets') to send to the hardware. To forbid the compiler to optimize and change fields alignment, mark the structures as packed. Signed-off-by: Philippe Mathieu-Daudé --- include/block/nvme.h | 38

[PATCH v2 0/4] hw/block/nvme: Fix I/O BAR structure

2020-06-30 Thread Philippe Mathieu-Daudé
Improvements for the I/O BAR structure: - correct pmrmsc register size (Klaus) - pack structures - align to 4KB Philippe Mathieu-Daudé (4): hw/block/nvme: Update specification URL hw/block/nvme: Use QEMU_PACKED on hardware/packet structures hw/block/nvme: Fix pmrmsc register size

Re: [PATCH 1/4] migration: Prevent memleak by ...params_test_apply

2020-06-30 Thread Dr. David Alan Gilbert
* Max Reitz (mre...@redhat.com) wrote: > The created structure is not really a proper QAPI object, so we cannot > and will not free its members. Strings therein should therefore not be > duplicated, or we will leak them. > > Signed-off-by: Max Reitz > --- > migration/migration.c | 4 ++-- > 1

Re: [PATCH v9 02/34] qcow2: Convert qcow2_get_cluster_offset() into qcow2_get_host_offset()

2020-06-30 Thread Alberto Garcia
On Tue 30 Jun 2020 12:19:42 PM CEST, Max Reitz wrote: >> @@ -537,8 +542,6 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, >> uint64_t offset, >> bytes_needed = bytes_available; >> } >> >> -*cluster_offset = 0; >> - > > You drop this line without replacement now. That

Re: [PATCH v2 06/18] hw/block/nvme: Define trace events related to NS Types

2020-06-30 Thread Klaus Jensen
On Jun 18 06:34, Dmitry Fomichev wrote: > A few trace events are defined that are relevant to implementing > Namespace Types (NVMe TP 4056). > > Signed-off-by: Dmitry Fomichev Reviewed-by: Klaus Jensen > --- > hw/block/trace-events | 11 +++ > 1 file changed, 11 insertions(+) > >

[PATCH v6 11/15] hw/sd/sdcard: Constify sd_crc*()'s message argument

2020-06-30 Thread Philippe Mathieu-Daudé
CRC functions don't modify the buffer argument, make it const. Reviewed-by: Alistair Francis Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 0fd672357c..2238ba066d 100644 ---

Re: [PATCH v9 02/34] qcow2: Convert qcow2_get_cluster_offset() into qcow2_get_host_offset()

2020-06-30 Thread Max Reitz
On 28.06.20 13:02, Alberto Garcia wrote: > qcow2_get_cluster_offset() takes an (unaligned) guest offset and > returns the (aligned) offset of the corresponding cluster in the qcow2 > image. > > In practice none of the callers need to know where the cluster starts > so this patch makes the

[PATCH v6 10/15] hw/sd/sdcard: Simplify cmd_valid_while_locked()

2020-06-30 Thread Philippe Mathieu-Daudé
cmd_valid_while_locked() only needs to read SDRequest->cmd, pass it directly and make it const. Reviewed-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index

[PATCH v6 08/15] hw/sd/sdcard: Check address is in range

2020-06-30 Thread Philippe Mathieu-Daudé
As a defense, assert if the requested address is out of the card area. Suggested-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé --- v6: call sd_addr_to_wpnum on 'size - 1' in reset() --- hw/sd/sd.c | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git

[PATCH v6 13/15] hw/sd/sdcard: Correctly display the command name in trace events

2020-06-30 Thread Philippe Mathieu-Daudé
Some ACMD were incorrectly displayed. Fix by remembering if we are processing a ACMD (with current_cmd_is_acmd) and add the sd_current_cmd_name() helper, which display to correct name regardless it is a CMD or ACMD. Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 17 ++--- 1

[PATCH v6 15/15] hw/sd/sdcard: Simplify realize() a bit

2020-06-30 Thread Philippe Mathieu-Daudé
We don't need to check if sd->blk is set twice. Reviewed-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 7f973f6763..e1bba887b2 100644 --- a/hw/sd/sd.c +++

[PATCH v6 05/15] hw/sd/sdcard: Do not switch to ReceivingData if address is invalid

2020-06-30 Thread Philippe Mathieu-Daudé
Only move the state machine to ReceivingData if there is no pending error. This avoids later OOB access while processing commands queued. "SD Specifications Part 1 Physical Layer Simplified Spec. v3.01" 4.3.3 Data Read Read command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR

[PATCH v6 04/15] hw/sd/sdcard: Use the HWBLOCK_SIZE definition

2020-06-30 Thread Philippe Mathieu-Daudé
Replace the following different uses of the same value by the same HWBLOCK_SIZE definition: - 512 (magic value) - 0x200 (magic value) - 1 << HWBLOCK_SHIFT Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff

[PATCH v6 03/15] hw/sd/sdcard: Move some definitions to use them earlier

2020-06-30 Thread Philippe Mathieu-Daudé
Move some definitions to use them earlier. Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index cac8d7d828..4816b4a462 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -80,6 +80,12 @@ enum

[PATCH v6 07/15] hw/sd/sdcard: Initialize constant values first

2020-06-30 Thread Philippe Mathieu-Daudé
Reorder initialization code, constant values first. Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 32 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 871c30a67f..22392e5084 100644 --- a/hw/sd/sd.c +++

[PATCH v6 14/15] hw/sd/sdcard: Display offset in read/write_data() trace events

2020-06-30 Thread Philippe Mathieu-Daudé
Having 'base address' and 'relative offset' displayed separately is more helpful than the absolute address. Reviewed-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 8 hw/sd/trace-events | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff

[PATCH v6 02/15] hw/sd/sdcard: Update coding style to make checkpatch.pl happy

2020-06-30 Thread Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé To make the next commit easier to review, clean this code first. Reviewed-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c

[PATCH v6 12/15] hw/sd/sdcard: Make iolen unsigned

2020-06-30 Thread Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé I/O request length can not be negative. Signed-off-by: Philippe Mathieu-Daudé --- v4: Use uint32_t (pm215) --- hw/sd/sd.c | 2 +- hw/sd/trace-events | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index

[PATCH 10/10] hw/block/nvme: support reset/finish recommended limits

2020-06-30 Thread Klaus Jensen
Add the rrl and frl device parameters. The parameters specify the number of seconds before the device may perform an internal operation to "clear" the Reset Zone Recommended and Finish Zone Recommended attributes respectively. When the attibutes are set is governed by the rrld and frld parameters

[PATCH v6 01/15] MAINTAINERS: Cc qemu-block mailing list

2020-06-30 Thread Philippe Mathieu-Daudé
From: Philippe Mathieu-Daudé We forgot to include the qemu-block mailing list while adding this section in commit 076a0fc32a7. Fix this. Suggested-by: Paolo Bonzini Signed-off-by: Philippe Mathieu-Daudé --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS

[PATCH v6 09/15] hw/sd/sdcard: Update the SDState documentation

2020-06-30 Thread Philippe Mathieu-Daudé
Add more descriptive comments to keep a clear separation between static property vs runtime changeable. Suggested-by: Peter Maydell Reviewed-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git

[PATCH v6 06/15] hw/sd/sdcard: Restrict Class 6 commands to SCSD cards

2020-06-30 Thread Philippe Mathieu-Daudé
Only SCSD cards support Class 6 (Block Oriented Write Protection) commands. "SD Specifications Part 1 Physical Layer Simplified Spec. v3.01" 4.3.14 Command Functional Difference in Card Capacity Types * Write Protected Group SDHC and SDXC do not support write-protected groups. Issuing

[PATCH v6 00/15] hw/sd/sdcard: Fix CVE-2020-13253 & cleanups

2020-06-30 Thread Philippe Mathieu-Daudé
Patches 5 & 6 fix CVE-2020-13253. The rest are (accumulated) cleanups. Since v5: Fix incorrect use of sd_addr_to_wpnum() in sd_reset() Missing review: [PATCH 01/15] MAINTAINERS: Cc qemu-block mailing list [PATCH 03/15] hw/sd/sdcard: Move some definitions to use them [PATCH 04/15] hw/sd/sdcard:

[PATCH 02/10] hw/block/nvme: add zns specific fields and types

2020-06-30 Thread Klaus Jensen
Add new fields, types and data structures for TP 4053 ("Zoned Namespaces"). Signed-off-by: Klaus Jensen --- include/block/nvme.h | 186 +-- 1 file changed, 180 insertions(+), 6 deletions(-) diff --git a/include/block/nvme.h b/include/block/nvme.h index

[PATCH 08/10] hw/block/nvme: allow open to close transitions by controller

2020-06-30 Thread Klaus Jensen
Allow the controller to release open resources by transitioning implicitly and explicitly opened zones to closed. This is done using a naive "least recently opened" strategy. Some workloads may behave very badly with this, but for the purpose of testing how software deals with this it is

[PATCH 03/10] hw/block/nvme: add basic read/write for zoned namespaces

2020-06-30 Thread Klaus Jensen
This adds basic read and write for zoned namespaces. A zoned namespace is created by setting the iocs parameter to 0x2 and supplying a zero-sized blockdev for zone info persistent state (zns.zoneinfo parameter) and the zns.zcap parameter to specify the individual zone capacities. The namespace

[PATCH 05/10] hw/block/nvme: add the zone management send command

2020-06-30 Thread Klaus Jensen
Add the Zone Management Send command. Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 461 ++ hw/block/nvme.h | 4 + hw/block/trace-events | 12 ++ 3 files changed, 477 insertions(+) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index

[PATCH 09/10] hw/block/nvme: allow zone excursions

2020-06-30 Thread Klaus Jensen
Allow the controller to release active resources by transitioning zones to the full state. Signed-off-by: Klaus Jensen --- hw/block/nvme-ns.h| 2 + hw/block/nvme.c | 171 ++ hw/block/trace-events | 4 + include/block/nvme.h | 10 +++ 4

[PATCH 07/10] hw/block/nvme: track and enforce zone resources

2020-06-30 Thread Klaus Jensen
Move all zone transition rules to a single state machine that also manages zone resources. Signed-off-by: Klaus Jensen --- hw/block/nvme-ns.c | 17 ++- hw/block/nvme-ns.h | 7 ++ hw/block/nvme.c| 304 - 3 files changed, 242 insertions(+), 86

[PATCH 00/10] hw/block/nvme: namespace types and zoned namespaces

2020-06-30 Thread Klaus Jensen
From: Klaus Jensen Hi all, This series adds support for TP 4056 ("Namespace Types") and TP 4053 ("Zoned Namespaces") and is an alternative implementation to the one submitted by Dmitry[1]. While I don't want this to end up as a discussion about the merits of each version, I want to point out a

[PATCH 06/10] hw/block/nvme: add the zone append command

2020-06-30 Thread Klaus Jensen
Add the Zone Append command. Signed-off-by: Klaus Jensen --- hw/block/nvme.c | 106 ++ hw/block/nvme.h | 3 ++ hw/block/trace-events | 2 + 3 files changed, 111 insertions(+) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index

[PATCH 04/10] hw/block/nvme: add the zone management receive command

2020-06-30 Thread Klaus Jensen
Add the Zone Management Receive command. Signed-off-by: Klaus Jensen --- hw/block/nvme-ns.c| 33 +-- hw/block/nvme-ns.h| 9 ++- hw/block/nvme.c | 130 ++ hw/block/nvme.h | 6 ++ hw/block/trace-events | 1 +

[PATCH 01/10] hw/block/nvme: support I/O Command Sets

2020-06-30 Thread Klaus Jensen
From: Klaus Jensen Implement support for TP 4056 ("Namespace Types"). This adds the 'iocs' (I/O Command Set) device parameter to the nvme-ns device. Signed-off-by: Klaus Jensen --- block/nvme.c | 6 +- hw/block/nvme-ns.c| 24 +++-- hw/block/nvme-ns.h| 11 +-

Re: [PATCH v5 08/15] hw/sd/sdcard: Check address is in range

2020-06-30 Thread Philippe Mathieu-Daudé
On 6/26/20 7:43 PM, Philippe Mathieu-Daudé wrote: > On 6/26/20 6:40 PM, Philippe Mathieu-Daudé wrote: >> As a defense, assert if the requested address is out of the card area. >> >> Suggested-by: Peter Maydell >> Signed-off-by: Philippe Mathieu-Daudé >> --- >> hw/sd/sd.c | 18 ++

  1   2   >