Re: [RFC 2/5] block/nvme: Change size and alignment of IDENTIFY response buffer

[Philippe Mathieu-Daudé]

On 10/15/20 1:52 PM, Eric Auger wrote:
> In preparation of 64kB host page support, let's change the size
> and alignment of the IDENTIFY command response buffer so that
> the VFIO DMA MAP succeeds. We align on the host page size.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   block/nvme.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/block/nvme.c b/block/nvme.c
> index e3d96f20d0..088ff1825a 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -513,19 +513,21 @@ static void nvme_identify(BlockDriverState *bs, int 
> namespace, Error **errp)
>           .opcode = NVME_ADM_CMD_IDENTIFY,
>           .cdw10 = cpu_to_le32(0x1),
>       };
> +    size_t id_size = QEMU_ALIGN_UP(sizeof(*id), qemu_real_host_page_size);

Do we really need to ALIGN_UP if we then call qemu_try_memalign()?

>   
> -    id = qemu_try_memalign(s->page_size, sizeof(*id));
> +    id = qemu_try_memalign(qemu_real_host_page_size, id_size);
>       if (!id) {
>           error_setg(errp, "Cannot allocate buffer for identify response");
>           goto out;
>       }
> -    r = qemu_vfio_dma_map(s->vfio, id, sizeof(*id), true, &iova);
> +    r = qemu_vfio_dma_map(s->vfio, id, id_size, true, &iova);
>       if (r) {
>           error_setg(errp, "Cannot map buffer for DMA");
>           goto out;
>       }
>   
> -    memset(id, 0, sizeof(*id));
> +    memset(id, 0, id_size);
> +
>       cmd.dptr.prp1 = cpu_to_le64(iova);
>       if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) {
>           error_setg(errp, "Failed to identify controller");
> @@ -547,7 +549,7 @@ static void nvme_identify(BlockDriverState *bs, int 
> namespace, Error **errp)
>       s->supports_write_zeroes = !!(oncs & NVME_ONCS_WRITE_ZEROES);
>       s->supports_discard = !!(oncs & NVME_ONCS_DSM);
>   
> -    memset(id, 0, sizeof(*id));
> +    memset(id, 0, id_size);
>       cmd.cdw10 = 0;
>       cmd.nsid = cpu_to_le32(namespace);
>       if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) {