[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] hw/cxl: Multi-Region CXL Type-3 Devices (Volatile and Pe
From: |
Jonathan Cameron |
Subject: |
Re: [PATCH 2/2] hw/cxl: Multi-Region CXL Type-3 Devices (Volatile and Persistent) |
Date: |
Fri, 17 Feb 2023 16:16:17 +0000 |
On Tue, 31 Jan 2023 16:38:47 +0000
Jonathan Cameron via <qemu-devel@nongnu.org> wrote:
> From: Gregory Price <gourry.memverge@gmail.com>
>
> This commit enables each CXL Type-3 device to contain one volatile
> memory region and one persistent region.
>
> Two new properties have been added to cxl-type3 device initialization:
> [volatile-memdev] and [persistent-memdev]
>
> The existing [memdev] property has been deprecated and will default the
> memory region to a persistent memory region (although a user may assign
> the region to a ram or file backed region). It cannot be used in
> combination with the new [persistent-memdev] property.
>
> Partitioning volatile memory from persistent memory is not yet supported.
>
> Volatile memory is mapped at DPA(0x0), while Persistent memory is mapped
> at DPA(vmem->size), per CXL Spec 8.2.9.8.2.0 - Get Partition Info.
>
> Signed-off-by: Gregory Price <gregory.price@memverge.com>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
Hi Gregory,
I've added support for multiple HDM decoders and hence can now
test both volatile and non volatile on same device.
It very nearly all works. With one exception which is I couldn't
poke the first byte of the non volatile region.
I think we have an off by one in a single check.
Interestingly it makes no difference when creating an FS on top
(which was my standard test) so I only noticed when poking memory
addresses directly to sanity check the HDM decoder setup.
I'll roll a v2 if no one shouts out that I'm wrong.
Note that adding multiple HDM decoders massively increases
the number of test cases over what we had before to poke all the
corners so I may well be missing stuff. Hopefully can send an RFC
of that support out next week.
Jonathan
> -MemTxResult cxl_type3_read(PCIDevice *d, hwaddr host_addr, uint64_t *data,
> - unsigned size, MemTxAttrs attrs)
> +static int cxl_type3_hpa_to_as_and_dpa(CXLType3Dev *ct3d,
> + hwaddr host_addr,
> + unsigned int size,
> + AddressSpace **as,
> + uint64_t *dpa_offset)
> {
> - CXLType3Dev *ct3d = CXL_TYPE3(d);
> - uint64_t dpa_offset;
> - MemoryRegion *mr;
> + MemoryRegion *vmr = NULL, *pmr = NULL;
>
> - /* TODO support volatile region */
> - mr = host_memory_backend_get_memory(ct3d->hostmem);
> - if (!mr) {
> - return MEMTX_ERROR;
> + if (ct3d->hostvmem) {
> + vmr = host_memory_backend_get_memory(ct3d->hostvmem);
> + }
> + if (ct3d->hostpmem) {
> + pmr = host_memory_backend_get_memory(ct3d->hostpmem);
> }
>
> - if (!cxl_type3_dpa(ct3d, host_addr, &dpa_offset)) {
> - return MEMTX_ERROR;
> + if (!vmr && !pmr) {
> + return -ENODEV;
> + }
> +
> + if (!cxl_type3_dpa(ct3d, host_addr, dpa_offset)) {
> + return -EINVAL;
> + }
> +
> + if (*dpa_offset > int128_get64(ct3d->cxl_dstate.mem_size)) {
> + return -EINVAL;
> + }
> +
> + if (vmr) {
> + if (*dpa_offset <= int128_get64(vmr->size)) {
Off by one I think. <
> + *as = &ct3d->hostvmem_as;
> + } else {
> + *as = &ct3d->hostpmem_as;
> + *dpa_offset -= vmr->size;
> + }
> + } else {
> + *as = &ct3d->hostpmem_as;
> }
>
> - if (dpa_offset > int128_get64(mr->size)) {
> + return 0;
> +}