Re: [PATCH v2 2/5] hw/net:ftgmac100: support 64 bits dma dram address for AST2700
Hello Jamin,
I think that, first, we should introduce a container region. In this container
region would be mapped a sub region for the current set of registers. This
container region would be the one that the SoC maps as it is done today.
Then, in a second patch, we should introduce a extra sub region for the set of
new registers and map it at 0x100 in the container region.
It is close to what you did but it lacks an overall container region.
This container region should be sized as specified in the datasheet.
Do you mean to change as following?
ftgmac100.h
#define FTGMAC100_MEM_SIZE 0x200
I would use the total size of the MMIO aperture, 0x1000, because
this region is reserved for the MAC unit logic, which means it
could grow. That's minor.
#define FTGMAC100_NR_REGS 0x100
Value is fine.
However, the NR_REGS suffix is confusing. It is not a number of
registers but a MMIO aperture width. I would use a _MEM_SIZE suffix
instead. Could be FTGMAC100_REG_MEM_SIZE.
#define FTGMAC100_REGS_HIGH_OFFSET 0x100
#define FTGMAC100_NR_REGS_HIGH 0x100
Same here.
struct FTGMAC100State {
MemoryRegion iomem_container;
MemoryRegion iomem;
MemoryRegion iomem_high;
}
Ftgmac100.c
static void ftgmac100_realize(DeviceState *dev, Error **errp)
{
memory_region_init(&s->iomem_container, OBJECT(s),
TYPE_FTGMAC100 ".container", FTGMAC100_MEM_SIZE); -->
container size 0x200
sysbus_init_mmio(sbd, &s->iomem_container);
memory_region_init_io(&s->iomem, OBJECT(s), &ftgmac100_ops, s,
TYPE_FTGMAC100 ".regs", FTGMAC100_NR_REGS); -->
current register 0x0-0xff
memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);
if (s-> dma64) {
memory_region_init_io(&s->iomem_high, OBJECT(s), &ftgmac100_high_ops,
s, TYPE_FTGMAC100 ".regs.high",
FTGMAC100_NR_REGS_HIGH); --> high register 0x100-0x1ff
memory_region_add_subregion(&s->iomem_container,
FTGMAC100_REGS_HIGH_OFFSET,
&s->iomem_high);
}
}
Looks good.
Thanks,
C.
PING: [PATCH v6 00/10] Support persistent reservation operations
Hi, The block layer codes has been reviewed by Stefan. Could you help me review the scsi and nvme layer codes. On 2024/6/27 10:53, 卢长奇 wrote: > the block layer code has been reviewed by Stefan.
RE: [PATCH v2 2/5] hw/net:ftgmac100: support 64 bits dma dram address for AST2700
Hi Cedric,
>
> On 7/3/24 10:16 AM, Jamin Lin wrote:
> > ASPEED AST2700 SOC is a 64 bits quad core CPUs (Cortex-a35) And the
> > base address of dram is "0x4 " which is 64bits address.
> >
> > It have "Normal Priority Transmit Ring Base Address Register
> > High(0x17C)", "High Priority Transmit Ring Base Address Register
> > High(0x184)" and "Receive Ring Base Address Register High(0x18C)" to
> > save the high part physical address of descriptor manager.
> > Ex: TX descriptor manager address [34:0] The "Normal Priority Transmit
> > Ring Base Address Register High(0x17C)"
> > bits [2:0] which corresponds the bits [34:32] of the 64 bits address
> > of the TX ring buffer address.
> > The "Normal Priority Transmit Ring Base Address Register(0x20)" bits
> > [31:0] which corresponds the bits [31:0] of the 64 bits address of the
> > TX ring buffer address.
> >
> > Besides, it have "TXDES 2" and "RXDES 2"
> > to save the high part physical address of packet buffer.
> > Ex: TX packet buffer address [34:0]
> > The "TXDES 2" bits [18:16] which corresponds the bits [34:32] of the
> > 64 bits address of the TX packet buffer address and "TXDES 3" bits
> > [31:0] which corresponds the bits [31:0] of the 64 bits address of the
> > TX packet buffer address.
> >
> > Update TX/RX ring and descriptor data type to uint64_t and supports
> > TX/RX ring, descriptor and packet buffers
> > 64 bits address for all ASPEED SOCs models.
> >
> > Incrementing the version of vmstate to 2.
> >
> > Introduce a new class(ftgmac100_high), class attribute and memop
> > handlers, the realize handler instantiate a new memory region and map
> > it on top of the current one to read/write FTGMAC100_*_HIGH regs.
> >
> > Signed-off-by: Jamin Lin
> > ---
> > hw/net/ftgmac100.c | 156
> -
> > include/hw/net/ftgmac100.h | 24 --
> > 2 files changed, 151 insertions(+), 29 deletions(-)
> >
> > diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c index
> > 4e88430b2f..3d13f54efc 100644
> > --- a/hw/net/ftgmac100.c
> > +++ b/hw/net/ftgmac100.c
> > @@ -56,6 +56,16 @@
> > #define FTGMAC100_PHYDATA 0x64
> > #define FTGMAC100_FCR 0x68
> >
> > +/*
> > + * FTGMAC100 High registers
> > + *
> > + * values below are offset by - FTGMAC100_HIGH_OFFSET from datasheet
> > + * because its memory region is start at FTGMAC100_HIGH_OFFSET */
> > +#define FTGMAC100_NPTXR_BADR_HIGH (0x17C -
> FTGMAC100_HIGH_OFFSET)
> > +#define FTGMAC100_HPTXR_BADR_HIGH (0x184 -
> FTGMAC100_HIGH_OFFSET)
> > +#define FTGMAC100_RXR_BADR_HIGH (0x18C -
> FTGMAC100_HIGH_OFFSET)
> > +
> > /*
> >* Interrupt status register & interrupt enable register
> >*/
> > @@ -165,6 +175,8 @@
> > #define FTGMAC100_TXDES1_TX2FIC (1 << 30)
> > #define FTGMAC100_TXDES1_TXIC(1 << 31)
> >
> > +#define FTGMAC100_TXDES2_TXBUF_BADR_HI(x) (((x) >> 16) & 0x7)
> > +
> > /*
> >* Receive descriptor
> >*/
> > @@ -198,13 +210,15 @@
> > #define FTGMAC100_RXDES1_UDP_CHKSUM_ERR (1 << 26)
> > #define FTGMAC100_RXDES1_IP_CHKSUM_ERR (1 << 27)
> >
> > +#define FTGMAC100_RXDES2_RXBUF_BADR_HI(x) (((x) >> 16) & 0x7)
> > +
> > /*
> >* Receive and transmit Buffer Descriptor
> >*/
> > typedef struct {
> > uint32_tdes0;
> > uint32_tdes1;
> > -uint32_tdes2;/* not used by HW */
> > +uint32_tdes2;/* used by HW high address */
> > uint32_tdes3;
> > } FTGMAC100Desc;
> >
> > @@ -515,12 +529,14 @@ out:
> > return frame_size;
> > }
> >
> > -static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
> > -uint32_t tx_descriptor)
> > +static void ftgmac100_do_tx(FTGMAC100State *s, uint64_t tx_ring,
> > +uint64_t tx_descriptor)
> > {
> > +FTGMAC100Class *fc = FTGMAC100_GET_CLASS(s);
> > int frame_size = 0;
> > uint8_t *ptr = s->frame;
> > -uint32_t addr = tx_descriptor;
> > +uint64_t addr = tx_descriptor;
> > +uint64_t buf_addr = 0;
>
> To ease reading, I would extract in a standalone patch the part converting
> addresses to 64 bits.
Will create a new patch
>
> > uint32_t flags = 0;
> >
> > while (1) {
> > @@ -559,7 +575,12 @@ static void ftgmac100_do_tx(FTGMAC100State *s,
> uint32_t tx_ring,
> > len = sizeof(s->frame) - frame_size;
> > }
> >
> > -if (dma_memory_read(&address_space_memory, bd.des3,
> > +buf_addr = bd.des3;
> > +if (fc->is_dma64) {
> > +buf_addr = deposit64(buf_addr, 32, 32,
> > +
> FTGMAC100_TXDES2_TXBUF_BADR_HI(bd.des2));
> > +}
> > +if (dma_memory_read(&address_space_memory, buf_addr,
> > ptr, len, MEMTXATTRS_UNSPECIFIED)) {
> > qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to read
> packet @ 0x%x\n",
> > __func__, bd.des3); @@ -726,9
[PULL v3 80/85] pcie_sriov: Ensure VF function number does not overflow
From: Akihiko Odaki
pci_new() aborts when creating a VF with a function number equals to or
is greater than PCI_DEVFN_MAX.
Signed-off-by: Akihiko Odaki
Message-Id: <20240627-reuse-v10-5-7ca0b8ed3...@daynix.com>
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
---
docs/pcie_sriov.txt | 8 +---
include/hw/pci/pcie_sriov.h | 5 +++--
hw/net/igb.c| 13 ++---
hw/nvme/ctrl.c | 24
hw/pci/pcie_sriov.c | 19 +--
5 files changed, 51 insertions(+), 18 deletions(-)
diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
index a47aad0bfa..ab2142807f 100644
--- a/docs/pcie_sriov.txt
+++ b/docs/pcie_sriov.txt
@@ -52,9 +52,11 @@ setting up a BAR for a VF.
...
/* Add and initialize the SR/IOV capability */
- pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
- vf_devid, initial_vfs, total_vfs,
- fun_offset, stride);
+ if (!pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
+ vf_devid, initial_vfs, total_vfs,
+ fun_offset, stride, errp)) {
+ return;
+ }
/* Set up individual VF BARs (parameters as for normal BARs) */
pcie_sriov_pf_init_vf_bar( ... )
diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
index 450cbef6c2..aa704e8f9d 100644
--- a/include/hw/pci/pcie_sriov.h
+++ b/include/hw/pci/pcie_sriov.h
@@ -27,10 +27,11 @@ typedef struct PCIESriovVF {
uint16_t vf_number; /* Logical VF number of this function */
} PCIESriovVF;
-void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
+bool pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
const char *vfname, uint16_t vf_dev_id,
uint16_t init_vfs, uint16_t total_vfs,
-uint16_t vf_offset, uint16_t vf_stride);
+uint16_t vf_offset, uint16_t vf_stride,
+Error **errp);
void pcie_sriov_pf_exit(PCIDevice *dev);
/* Set up a VF bar in the SR/IOV bar area */
diff --git a/hw/net/igb.c b/hw/net/igb.c
index b92bba402e..b6ca2f1b8a 100644
--- a/hw/net/igb.c
+++ b/hw/net/igb.c
@@ -446,9 +446,16 @@ static void igb_pci_realize(PCIDevice *pci_dev, Error
**errp)
pcie_ari_init(pci_dev, 0x150);
-pcie_sriov_pf_init(pci_dev, IGB_CAP_SRIOV_OFFSET, TYPE_IGBVF,
-IGB_82576_VF_DEV_ID, IGB_MAX_VF_FUNCTIONS, IGB_MAX_VF_FUNCTIONS,
-IGB_VF_OFFSET, IGB_VF_STRIDE);
+if (!pcie_sriov_pf_init(pci_dev, IGB_CAP_SRIOV_OFFSET,
+TYPE_IGBVF, IGB_82576_VF_DEV_ID,
+IGB_MAX_VF_FUNCTIONS, IGB_MAX_VF_FUNCTIONS,
+IGB_VF_OFFSET, IGB_VF_STRIDE,
+errp)) {
+pcie_cap_exit(pci_dev);
+igb_cleanup_msix(s);
+msi_uninit(pci_dev);
+return;
+}
pcie_sriov_pf_init_vf_bar(pci_dev, IGBVF_MMIO_BAR_IDX,
PCI_BASE_ADDRESS_MEM_TYPE_64 | PCI_BASE_ADDRESS_MEM_PREFETCH,
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 127c3d2383..066389e391 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8048,7 +8048,8 @@ out:
return pow2ceil(bar_size);
}
-static void nvme_init_sriov(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t offset)
+static bool nvme_init_sriov(NvmeCtrl *n, PCIDevice *pci_dev, uint16_t offset,
+Error **errp)
{
uint16_t vf_dev_id = n->params.use_intel_id ?
PCI_DEVICE_ID_INTEL_NVME : PCI_DEVICE_ID_REDHAT_NVME;
@@ -8057,12 +8058,17 @@ static void nvme_init_sriov(NvmeCtrl *n, PCIDevice
*pci_dev, uint16_t offset)
le16_to_cpu(cap->vifrsm),
NULL, NULL);
-pcie_sriov_pf_init(pci_dev, offset, "nvme", vf_dev_id,
- n->params.sriov_max_vfs, n->params.sriov_max_vfs,
- NVME_VF_OFFSET, NVME_VF_STRIDE);
+if (!pcie_sriov_pf_init(pci_dev, offset, "nvme", vf_dev_id,
+n->params.sriov_max_vfs, n->params.sriov_max_vfs,
+NVME_VF_OFFSET, NVME_VF_STRIDE,
+errp)) {
+return false;
+}
pcie_sriov_pf_init_vf_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
PCI_BASE_ADDRESS_MEM_TYPE_64, bar_size);
+
+return true;
}
static int nvme_add_pm_capability(PCIDevice *pci_dev, uint8_t offset)
@@ -8155,6 +8161,12 @@ static bool nvme_init_pci(NvmeCtrl *n, PCIDevice
*pci_dev, Error **errp)
return false;
}
+if (!pci_is_vf(pci_dev) && n->params.sriov_max_vfs &&
+!nvme_init_sriov(n, pci_dev, 0x120, errp)) {
+msix_uninit(pci_dev, &n->bar0, &n->bar0);
+return false;
+}
+
nvme_update_msixcap_ts(pci_dev, n->conf_msix_qsize);
if (n->params.cmb_size_mb) {
@@ -8165,10 +8177,6 @@ sta
Re: [PATCH v8 00/13] vhost-user: support any POSIX system (tested on macOS, FreeBSD, OpenBSD)
On Tue, Jun 18, 2024 at 12:00:30PM +0200, Stefano Garzarella wrote: > As discussed with Michael and Markus [1], this version also includes the patch > on which v7 depended to simplify the merge in Michael's tree. > > The series is all reviewed, so if there are no new changes required, I would > ask to merge it. I dropped patches 9 and 10 for now since otherwise make vm-build-freebsd fails. Pls figure it out and resend just 9 and 10. > [1] > https://patchew.org/QEMU/20240612130140.63004-1-sgarz...@redhat.com/#vabzv4z6g3dd5yndvpmwktcfgbqrdg7qk2e5se6zuflrhss723@dws4vrzen6cs > > Thanks, > Stefano > > Changelog > > v1: https://patchew.org/QEMU/20240228114759.44758-1-sgarz...@redhat.com/ > v2: https://patchew.org/QEMU/20240326133936.125332-1-sgarz...@redhat.com/ > v3: https://patchew.org/QEMU/20240404122330.92710-1-sgarz...@redhat.com/ > v4: https://patchew.org/QEMU/20240508074457.12367-1-sgarz...@redhat.com/ > v5: https://patchew.org/QEMU/20240523145522.313012-1-sgarz...@redhat.com/ > v6: https://patchew.org/QEMU/20240528103543.145412-1-sgarz...@redhat.com/ > v7: https://patchew.org/QEMU/20240612130140.63004-1-sgarz...@redhat.com/ > v8: > - Included the dependent patch in this series > https://patchew.org/QEMU/20240611130231.83152-1-sgarz...@redhat.com/ > - Fixed QAPI documentation about share option [Markus] > > Description > > The vhost-user protocol is not really Linux-specific, so let's try support > QEMU's frontends and backends (including libvhost-user) in any POSIX system > with this series. The main use case is to be able to use virtio devices that > we don't have built-in in QEMU (e.g. virtiofsd, vhost-user-vsock, etc.) even > in non-Linux systems. > > The first 5 patches are more like fixes discovered at runtime on macOS or > FreeBSD that could go even independently of this series. > > Patches 6, 7, 8, 9 enable building of frontends and backends (including > libvhost-user) with associated code changes to succeed in compilation. > > Patch 10 adds `memory-backend-shm` that uses the POSIX shm_open() API to > create shared memory which is identified by an fd that can be shared with > vhost-user backends. This is useful on those systems (like macOS) where > we don't have memfd_create() or special filesystems like "/dev/shm". > > Patches 11 and 12 use `memory-backend-shm` in some vhost-user tests. > > Maybe the first 5 patches can go separately, but I only discovered those > problems after testing patches 6 - 9, so I have included them in this series > for now. Please let me know if you prefer that I send them separately. > > I tested this series using vhost-user-blk and QSD on macOS Sonoma 14.4 > (aarch64), FreeBSD 14 (x86_64), OpenBSD 7.4 (x86_64), and Fedora 40 (x86_64) > in this way: > > - Start vhost-user-blk or QSD (same commands for all systems) > > vhost-user-blk -s /tmp/vhost.socket \ > -b Fedora-Cloud-Base-39-1.5.x86_64.raw > > qemu-storage-daemon \ > --blockdev > file,filename=Fedora-Cloud-Base-39-1.5.x86_64.qcow2,node-name=file \ > --blockdev qcow2,file=file,node-name=qcow2 \ > --export > vhost-user-blk,addr.type=unix,addr.path=/tmp/vhost.socket,id=vub,num-queues=1,node-name=qcow2,writable=on > > - macOS (aarch64): start QEMU (using hvf accelerator) > > qemu-system-aarch64 -smp 2 -cpu host -M virt,accel=hvf,memory-backend=mem \ > -drive > file=./build/pc-bios/edk2-aarch64-code.fd,if=pflash,format=raw,readonly=on \ > -device virtio-net-device,netdev=net0 -netdev user,id=net0 \ > -device ramfb -device usb-ehci -device usb-kbd \ > -object memory-backend-shm,id=mem,size=512M \ > -device vhost-user-blk-pci,num-queues=1,disable-legacy=on,chardev=char0 \ > -chardev socket,id=char0,path=/tmp/vhost.socket > > - FreeBSD/OpenBSD (x86_64): start QEMU (no accelerators available) > > qemu-system-x86_64 -smp 2 -M q35,memory-backend=mem \ > -object memory-backend-shm,id=mem,size="512M" \ > -device vhost-user-blk-pci,num-queues=1,chardev=char0 \ > -chardev socket,id=char0,path=/tmp/vhost.socket > > - Fedora (x86_64): start QEMU (using kvm accelerator) > > qemu-system-x86_64 -smp 2 -M q35,accel=kvm,memory-backend=mem \ > -object memory-backend-shm,size="512M" \ > -device vhost-user-blk-pci,num-queues=1,chardev=char0 \ > -chardev socket,id=char0,path=/tmp/vhost.socket > > Branch pushed (and CI started) at > https://gitlab.com/sgarzarella/qemu/-/tree/macos-vhost-user?ref_type=heads > > Stefano Garzarella (13): > qapi: clarify that the default is backend dependent > libvhost-user: set msg.msg_control to NULL when it is empty > libvhost-user: fail vu_message_write() if sendmsg() is failing > libvhost-user: mask F_INFLIGHT_SHMFD if memfd is not supported > vhost-user-server: do not set memory fd non-blocking > contrib/vhost-user-blk: fix bind() using the right size of the address > contrib/vhost-user-*: use QEMU bswap helper functions > vhost-user: enable frontends on any POSIX system > libvho
[PULL v3 15/85] vhost-user: fix lost reconnect again
From: Li Feng
When the vhost-user is reconnecting to the backend, and if the vhost-user fails
at the get_features in vhost_dev_init(), then the reconnect will fail
and it will not be retriggered forever.
The reason is:
When the vhost-user fail at get_features, the vhost_dev_cleanup will be called
immediately.
vhost_dev_cleanup calls 'memset(hdev, 0, sizeof(struct vhost_dev))'.
The reconnect path is:
vhost_user_blk_event
vhost_user_async_close(.. vhost_user_blk_disconnect ..)
qemu_chr_fe_set_handlers <- clear the notifier callback
schedule vhost_user_async_close_bh
The vhost->vdev is null, so the vhost_user_blk_disconnect will not be
called, then the event fd callback will not be reinstalled.
We need to ensure that even if vhost_dev_init initialization fails, the event
handler still needs to be reinstalled when s->connected is false.
All vhost-user devices have this issue, including vhost-user-blk/scsi.
Fixes: 71e076a07d ("hw/virtio: generalise CHR_EVENT_CLOSED handling")
Signed-off-by: Li Feng
Message-Id: <20240516025753.130171-3-fen...@smartx.com>
Reviewed-by: Raphael Norwitz
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
---
hw/block/vhost-user-blk.c | 3 ++-
hw/scsi/vhost-user-scsi.c | 3 ++-
hw/virtio/vhost-user-base.c | 3 ++-
hw/virtio/vhost-user.c | 10 +-
4 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 15cc24d017..fdbc30b9ce 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -354,7 +354,7 @@ static void vhost_user_blk_disconnect(DeviceState *dev)
VHostUserBlk *s = VHOST_USER_BLK(vdev);
if (!s->connected) {
-return;
+goto done;
}
s->connected = false;
@@ -362,6 +362,7 @@ static void vhost_user_blk_disconnect(DeviceState *dev)
vhost_dev_cleanup(&s->dev);
+done:
/* Re-instate the event handler for new connections */
qemu_chr_fe_set_handlers(&s->chardev, NULL, NULL, vhost_user_blk_event,
NULL, dev, NULL, true);
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 421cd654f8..cc91ade525 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -182,7 +182,7 @@ static void vhost_user_scsi_disconnect(DeviceState *dev)
VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
if (!s->connected) {
-return;
+goto done;
}
s->connected = false;
@@ -190,6 +190,7 @@ static void vhost_user_scsi_disconnect(DeviceState *dev)
vhost_dev_cleanup(&vsc->dev);
+done:
/* Re-instate the event handler for new connections */
qemu_chr_fe_set_handlers(&vs->conf.chardev, NULL, NULL,
vhost_user_scsi_event, NULL, dev, NULL, true);
diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index 4b54255682..11e72b1e3b 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -225,13 +225,14 @@ static void vub_disconnect(DeviceState *dev)
VHostUserBase *vub = VHOST_USER_BASE(vdev);
if (!vub->connected) {
-return;
+goto done;
}
vub->connected = false;
vub_stop(vdev);
vhost_dev_cleanup(&vub->vhost_dev);
+done:
/* Re-instate the event handler for new connections */
qemu_chr_fe_set_handlers(&vub->chardev,
NULL, NULL, vub_event,
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index c929097e87..c407ea8939 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2781,16 +2781,8 @@ typedef struct {
static void vhost_user_async_close_bh(void *opaque)
{
VhostAsyncCallback *data = opaque;
-struct vhost_dev *vhost = data->vhost;
-/*
- * If the vhost_dev has been cleared in the meantime there is
- * nothing left to do as some other path has completed the
- * cleanup.
- */
-if (vhost->vdev) {
-data->cb(data->dev);
-}
+data->cb(data->dev);
g_free(data);
}
--
MST
[PULL v3 14/85] Revert "vhost-user: fix lost reconnect"
From: Li Feng
This reverts commit f02a4b8e6431598612466f76aac64ab492849abf.
Since the current patch cannot completely fix the lost reconnect
problem, there is a scenario that is not considered:
- When the virtio-blk driver is removed from the guest os,
s->connected has no chance to be set to false, resulting in
subsequent reconnection not being executed.
The next patch will completely fix this issue with a better approach.
Signed-off-by: Li Feng
Message-Id: <20240516025753.130171-2-fen...@smartx.com>
Reviewed-by: Raphael Norwitz
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
---
include/hw/virtio/vhost-user.h | 3 +--
hw/block/vhost-user-blk.c | 2 +-
hw/scsi/vhost-user-scsi.c | 3 +--
hw/virtio/vhost-user-base.c| 2 +-
hw/virtio/vhost-user.c | 10 ++
5 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index d7c09ffd34..324cd8663a 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -108,7 +108,6 @@ typedef void (*vu_async_close_fn)(DeviceState *cb);
void vhost_user_async_close(DeviceState *d,
CharBackend *chardev, struct vhost_dev *vhost,
-vu_async_close_fn cb,
-IOEventHandler *event_cb);
+vu_async_close_fn cb);
#endif
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index bc2677dbef..15cc24d017 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -385,7 +385,7 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent
event)
case CHR_EVENT_CLOSED:
/* defer close until later to avoid circular close */
vhost_user_async_close(dev, &s->chardev, &s->dev,
- vhost_user_blk_disconnect,
vhost_user_blk_event);
+ vhost_user_blk_disconnect);
break;
case CHR_EVENT_BREAK:
case CHR_EVENT_MUX_IN:
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 0b050805a8..421cd654f8 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -215,8 +215,7 @@ static void vhost_user_scsi_event(void *opaque,
QEMUChrEvent event)
case CHR_EVENT_CLOSED:
/* defer close until later to avoid circular close */
vhost_user_async_close(dev, &vs->conf.chardev, &vsc->dev,
- vhost_user_scsi_disconnect,
- vhost_user_scsi_event);
+ vhost_user_scsi_disconnect);
break;
case CHR_EVENT_BREAK:
case CHR_EVENT_MUX_IN:
diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index a83167191e..4b54255682 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -254,7 +254,7 @@ static void vub_event(void *opaque, QEMUChrEvent event)
case CHR_EVENT_CLOSED:
/* defer close until later to avoid circular close */
vhost_user_async_close(dev, &vub->chardev, &vub->vhost_dev,
- vub_disconnect, vub_event);
+ vub_disconnect);
break;
case CHR_EVENT_BREAK:
case CHR_EVENT_MUX_IN:
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index cdf9af4a4b..c929097e87 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2776,7 +2776,6 @@ typedef struct {
DeviceState *dev;
CharBackend *cd;
struct vhost_dev *vhost;
-IOEventHandler *event_cb;
} VhostAsyncCallback;
static void vhost_user_async_close_bh(void *opaque)
@@ -2791,10 +2790,7 @@ static void vhost_user_async_close_bh(void *opaque)
*/
if (vhost->vdev) {
data->cb(data->dev);
-} else if (data->event_cb) {
-qemu_chr_fe_set_handlers(data->cd, NULL, NULL, data->event_cb,
- NULL, data->dev, NULL, true);
- }
+}
g_free(data);
}
@@ -2806,8 +2802,7 @@ static void vhost_user_async_close_bh(void *opaque)
*/
void vhost_user_async_close(DeviceState *d,
CharBackend *chardev, struct vhost_dev *vhost,
-vu_async_close_fn cb,
-IOEventHandler *event_cb)
+vu_async_close_fn cb)
{
if (!runstate_check(RUN_STATE_SHUTDOWN)) {
/*
@@ -2823,7 +2818,6 @@ void vhost_user_async_close(DeviceState *d,
data->dev = d;
data->cd = chardev;
data->vhost = vhost;
-data->event_cb = event_cb;
/* Disable any further notifications on the chardev */
qemu_chr_fe_set_handlers(chardev,
--
MST
[PULL v3 08/85] vhost/vhost-user: Add VIRTIO_F_NOTIFICATION_DATA to vhost feature bits
From: Jonah Palmer
Add support for the VIRTIO_F_NOTIFICATION_DATA feature across a variety
of vhost devices.
The inclusion of VIRTIO_F_NOTIFICATION_DATA in the feature bits arrays
for these devices ensures that the backend is capable of offering and
providing support for this feature, and that it can be disabled if the
backend does not support it.
Tested-by: Lei Yang
Reviewed-by: Eugenio Pérez
Signed-off-by: Jonah Palmer
Message-Id: <20240315165557.26942-6-jonah.pal...@oracle.com>
Acked-by: Srujana Challa
Reviewed-by: Michael S. Tsirkin
Signed-off-by: Michael S. Tsirkin
---
hw/block/vhost-user-blk.c| 1 +
hw/net/vhost_net.c | 2 ++
hw/scsi/vhost-scsi.c | 1 +
hw/scsi/vhost-user-scsi.c| 1 +
hw/virtio/vhost-user-fs.c| 2 +-
hw/virtio/vhost-user-vsock.c | 1 +
net/vhost-vdpa.c | 1 +
7 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9e6bbc6950..bc2677dbef 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -51,6 +51,7 @@ static const int user_feature_bits[] = {
VIRTIO_F_RING_PACKED,
VIRTIO_F_IOMMU_PLATFORM,
VIRTIO_F_RING_RESET,
+VIRTIO_F_NOTIFICATION_DATA,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index fd1a93701a..18898afe81 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -48,6 +48,7 @@ static const int kernel_feature_bits[] = {
VIRTIO_F_IOMMU_PLATFORM,
VIRTIO_F_RING_PACKED,
VIRTIO_F_RING_RESET,
+VIRTIO_F_NOTIFICATION_DATA,
VIRTIO_NET_F_HASH_REPORT,
VHOST_INVALID_FEATURE_BIT
};
@@ -55,6 +56,7 @@ static const int kernel_feature_bits[] = {
/* Features supported by others. */
static const int user_feature_bits[] = {
VIRTIO_F_NOTIFY_ON_EMPTY,
+VIRTIO_F_NOTIFICATION_DATA,
VIRTIO_RING_F_INDIRECT_DESC,
VIRTIO_RING_F_EVENT_IDX,
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index ae26bc19a4..3d5fe0994d 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -38,6 +38,7 @@ static const int kernel_feature_bits[] = {
VIRTIO_RING_F_EVENT_IDX,
VIRTIO_SCSI_F_HOTPLUG,
VIRTIO_F_RING_RESET,
+VIRTIO_F_NOTIFICATION_DATA,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index a63b1f4948..0b050805a8 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -36,6 +36,7 @@ static const int user_feature_bits[] = {
VIRTIO_RING_F_EVENT_IDX,
VIRTIO_SCSI_F_HOTPLUG,
VIRTIO_F_RING_RESET,
+VIRTIO_F_NOTIFICATION_DATA,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
index cca2cd41be..ae48cc1c96 100644
--- a/hw/virtio/vhost-user-fs.c
+++ b/hw/virtio/vhost-user-fs.c
@@ -33,7 +33,7 @@ static const int user_feature_bits[] = {
VIRTIO_F_RING_PACKED,
VIRTIO_F_IOMMU_PLATFORM,
VIRTIO_F_RING_RESET,
-
+VIRTIO_F_NOTIFICATION_DATA,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/hw/virtio/vhost-user-vsock.c b/hw/virtio/vhost-user-vsock.c
index 9431b9792c..802b44a07d 100644
--- a/hw/virtio/vhost-user-vsock.c
+++ b/hw/virtio/vhost-user-vsock.c
@@ -21,6 +21,7 @@ static const int user_feature_bits[] = {
VIRTIO_RING_F_INDIRECT_DESC,
VIRTIO_RING_F_EVENT_IDX,
VIRTIO_F_NOTIFY_ON_EMPTY,
+VIRTIO_F_NOTIFICATION_DATA,
VHOST_INVALID_FEATURE_BIT
};
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index eda714d1a4..daa38428c5 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -62,6 +62,7 @@ const int vdpa_feature_bits[] = {
VIRTIO_F_RING_PACKED,
VIRTIO_F_RING_RESET,
VIRTIO_F_VERSION_1,
+VIRTIO_F_NOTIFICATION_DATA,
VIRTIO_NET_F_CSUM,
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS,
VIRTIO_NET_F_CTRL_MAC_ADDR,
--
MST
Re: [PULL 4/4] block: Parse filenames only when explicitly requested
02.07.2024 19:39, Kevin Wolf wrote:
When handling image filenames from legacy options such as -drive or from
tools, these filenames are parsed for protocol prefixes, including for
the json:{} pseudo-protocol.
This behaviour is intended for filenames that come directly from the
command line and for backing files, which may come from the image file
itself. Higher level management tools generally take care to verify that
untrusted images don't contain a bad (or any) backing file reference;
'qemu-img info' is a suitable tool for this.
However, for other files that can be referenced in images, such as
qcow2 data files or VMDK extents, the string from the image file is
usually not verified by management tools - and 'qemu-img info' wouldn't
be suitable because in contrast to backing files, it already opens these
other referenced files. So here the string should be interpreted as a
literal local filename. More complex configurations need to be specified
explicitly on the command line or in QMP.
This patch changes bdrv_open_inherit() so that it only parses filenames
if a new parameter parse_filename is true. It is set for the top level
in bdrv_open(), for the file child and for the backing file child. All
other callers pass false and disable filename parsing this way.
I'm attaching backports of this change to 8.2 and 7.2 series.
Please check if the resulting backports are correct, or if they're needed
in the first place. Note: 7.2 lacks quite some locking in this area, eg
v8.0.0-2069-g8394c35ee148 "block: Fix AioContext locking in bdrv_open_child()"
v8.2.0-rc0-59-g6bc0bcc89f84 "block: Fix deadlocks in bdrv_graph_wrunlock()".
Thanks,
/mjt
--
GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24.
New key: rsa4096/61AD3D98ECDF2C8E 9D8B E14E 3F2A 9DD7 9199 28F1 61AD 3D98
ECDF 2C8E
Old key: rsa2048/457CE0A0804465C5 6EE1 95D1 886E 8FFB 810D 4324 457C E0A0
8044 65C5
Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt
From 0408443ecb8785a20652b529aa61f020e304112c Mon Sep 17 00:00:00 2001
From: Kevin Wolf
Date: Thu, 25 Apr 2024 14:56:02 +0200
Subject: [PATCH] block: Parse filenames only when explicitly requested
When handling image filenames from legacy options such as -drive or from
tools, these filenames are parsed for protocol prefixes, including for
the json:{} pseudo-protocol.
This behaviour is intended for filenames that come directly from the
command line and for backing files, which may come from the image file
itself. Higher level management tools generally take care to verify that
untrusted images don't contain a bad (or any) backing file reference;
'qemu-img info' is a suitable tool for this.
However, for other files that can be referenced in images, such as
qcow2 data files or VMDK extents, the string from the image file is
usually not verified by management tools - and 'qemu-img info' wouldn't
be suitable because in contrast to backing files, it already opens these
other referenced files. So here the string should be interpreted as a
literal local filename. More complex configurations need to be specified
explicitly on the command line or in QMP.
This patch changes bdrv_open_inherit() so that it only parses filenames
if a new parameter parse_filename is true. It is set for the top level
in bdrv_open(), for the file child and for the backing file child. All
other callers pass false and disable filename parsing this way.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Kevin Wolf
Reviewed-by: Eric Blake
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Hanna Czenczek
(cherry picked from commit 7ead946998610657d38d1a505d5f25300d4ca613)
Signed-off-by: Michael Tokarev
(Mjt: backport patch to 7.2, without:
v8.0.0-2069-g8394c35ee148 "block: Fix AioContext locking in bdrv_open_child()"
v8.1.0-801-gafdaeb9ea06e "block: Mark bdrv_attach_child() GRAPH_WRLOCK"
v8.2.0-rc0-59-g6bc0bcc89f84 "block: Fix deadlocks in bdrv_graph_wrunlock()"
v8.2.0-132-g6bc30f194985 "graph-lock: remove AioContext locking"
v8.2.0-133-gb49f4755c7fa "block: remove AioContext locking")
---
block.c | 76 +
1 file changed, 49 insertions(+), 27 deletions(-)
diff --git a/block.c b/block.c
index a18f052374..ea369a3fe5 100644
--- a/block.c
+++ b/block.c
@@ -85,6 +85,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
BlockDriverState *parent,
const BdrvChildClass *child_class,
BdrvChildRole child_role,
+ bool parse_filename,
Error **errp);
static bool bdrv_recurse_has_child(BlockDriverState *bs,
@@ -2051,7 +2052,8 @@ static void parse_json_protocol(QDict *options, const char **pfilename,
* block driver has been specified explicitly.
*/
static int bdrv_fill_options(QDict **options, const char *filename,
-
[PATCH 5/8] qapi: convert "Example" sections without titles
Use the no-option form of ".. qmp-example::" to convert any Examples
that do not have any form of caption or explanation whatsoever. Note
that in a few cases, example sections are split into two or more
separate example blocks. This is only done stylistically to create a
delineation between two or more logically independent examples.
See commit-3: "docs/qapidoc: create qmp-example directive", for a
detailed explanation of this custom directive syntax.
See commit+3: "qapi: remove "Example" doc section" for a detailed
explanation of why.
Signed-off-by: John Snow
---
qapi/acpi.json | 4 +--
qapi/block-core.json | 64 +---
qapi/block.json | 18 ++-
qapi/char.json | 24 +--
qapi/control.json| 8 ++---
qapi/dump.json | 8 ++---
qapi/machine-target.json | 2 +-
qapi/machine.json| 38
qapi/migration.json | 58 ++--
qapi/misc-target.json| 22 +++---
qapi/misc.json | 32 ++--
qapi/net.json| 20 +++--
qapi/pci.json| 2 +-
qapi/qdev.json | 10 ---
qapi/qom.json| 8 ++---
qapi/replay.json | 8 ++---
qapi/rocker.json | 8 ++---
qapi/run-state.json | 30 +--
qapi/tpm.json| 6 ++--
qapi/trace.json | 4 +--
qapi/transaction.json| 2 +-
qapi/ui.json | 34 ++---
qapi/vfio.json | 2 +-
qapi/virtio.json | 2 +-
qapi/yank.json | 4 +--
25 files changed, 216 insertions(+), 202 deletions(-)
diff --git a/qapi/acpi.json b/qapi/acpi.json
index aa4dbe57943..045dab6228b 100644
--- a/qapi/acpi.json
+++ b/qapi/acpi.json
@@ -111,7 +111,7 @@
#
# Since: 2.1
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "query-acpi-ospm-status" }
# <- { "return": [ { "device": "d1", "slot": "0", "slot-type": "DIMM",
"source": 1, "status": 0},
@@ -131,7 +131,7 @@
#
# Since: 2.1
#
-# Example:
+# .. qmp-example::
#
# <- { "event": "ACPI_DEVICE_OST",
# "data": { "info": { "device": "d1", "slot": "0",
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9ef23ec02ae..4e0f0395146 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -764,7 +764,7 @@
#
# Since: 0.14
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "query-block" }
# <- {
@@ -1168,7 +1168,7 @@
#
# Since: 0.14
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "query-blockstats" }
# <- {
@@ -1461,7 +1461,7 @@
#
# Since: 0.14
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "block_resize",
# "arguments": { "device": "scratch", "size": 1073741824 } }
@@ -1682,7 +1682,7 @@
#
# Since: 0.14
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "blockdev-snapshot-sync",
# "arguments": { "device": "ide-hd0",
@@ -1715,7 +1715,7 @@
#
# Since: 2.5
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "blockdev-add",
# "arguments": { "driver": "qcow2",
@@ -1861,7 +1861,7 @@
#
# Since: 1.3
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "block-commit",
# "arguments": { "device": "virtio0",
@@ -1899,7 +1899,7 @@
#
# Since: 1.6
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "drive-backup",
# "arguments": { "device": "drive0",
@@ -1925,7 +1925,7 @@
#
# Since: 2.3
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "blockdev-backup",
# "arguments": { "device": "src-id",
@@ -1949,7 +1949,7 @@
#
# Since: 2.0
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "query-named-block-nodes" }
# <- { "return": [ { "ro":false,
@@ -2130,7 +2130,7 @@
#
# Since: 1.3
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "drive-mirror",
# "arguments": { "device": "ide-hd0",
@@ -2307,7 +2307,7 @@
#
# Since: 2.4
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "block-dirty-bitmap-add",
# "arguments": { "node": "drive0", "name": "bitmap0" } }
@@ -2331,7 +2331,7 @@
#
# Since: 2.4
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "block-dirty-bitmap-remove",
# "arguments": { "node": "drive0", "name": "bitmap0" } }
@@ -2354,7 +2354,7 @@
#
# Since: 2.4
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "block-dirty-bitmap-clear",
# "arguments": { "node": "drive0", "name": "bitmap0" } }
@@ -2375,7 +2375,7 @@
#
# Since: 4.0
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "block-dirty-bitmap-enable",
# "arguments": { "node": "drive0", "name": "bitmap0" } }
@@ -2396,7 +2396,7 @@
#
# Since: 4.0
#
-# Example:
+# .. qmp-example::
#
# -> { "execute": "block-dirty-bitmap-disable",
# "arguments": { "node": "drive0", "name": "bitmap0" } }
@@ -242
[PATCH 8/8] qapi: remove "Example" doc section
Fully eliminate the "Example" sections in QAPI doc blocks now that they
have all been converted to arbitrary rST syntax using the
".. qmp-example::" directive. Update tests to match.
Migrating to the new syntax
---
The old "Example:" or "Examples:" section syntax is now caught as an
error, but "Example::" is stil permitted as explicit rST syntax for an
un-lexed, generic preformatted text block.
('Example' is not special in this case, any sentence that ends with "::"
will start an indented code block in rST.)
Arbitrary rST for Examples is now possible, but it's strongly
recommended that documentation authors use the ".. qmp-example::"
directive for consistent visual formatting in rendered HTML docs. The
":title:" directive option may be used to add extra information into the
title bar for the example. The ":annotated:" option can be used to write
arbitrary rST instead, with nested "::" blocks applying QMP formatting
where desired.
Other choices available are ".. code-block:: QMP" which will not create
an "Example:" box, or the short-form "::" code-block syntax which will
not apply QMP highlighting when used outside of the qmp-example
directive.
Why?
This patch has several benefits:
1. Example sections can now be written more arbitrarily, mixing
explanatory paragraphs and code blocks however desired.
2. Example sections can now use fully arbitrary rST.
3. All code blocks are now lexed and validated as QMP; increasing
usability of the docs and ensuring validity of example snippets.
(To some extent - This patch only gaurantees it lexes correctly, not
that it's valid under the JSON or QMP grammars. It will catch most
small mistakes, however.)
4. Each qmp-example can be titled or annotated independently without
bypassing the QMP lexer/validator.
(i.e. code blocks are now for *code* only, so we don't have to
sacrifice exposition for having lexically valid examples.)
NOTE: As with the "Notes" conversion patch, this patch (and those
preceding) may change the rendering order for Examples in the
current generator. The forthcoming qapidoc rewrite will fix this
by always generating documentation in source order.
Signed-off-by: John Snow
---
docs/devel/qapi-code-gen.rst| 58 -
scripts/qapi/parser.py | 10 +-
tests/qapi-schema/doc-good.json | 19 +++
tests/qapi-schema/doc-good.out | 26 ++-
tests/qapi-schema/doc-good.txt | 23 ++---
5 files changed, 98 insertions(+), 38 deletions(-)
diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index ae97b335cbf..2e10a3cbd69 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -899,7 +899,7 @@ Documentation markup
Documentation comments can use most rST markup. In particular,
-a ``::`` literal block can be used for examples::
+a ``::`` literal block can be used for pre-formatted text::
# ::
#
@@ -995,8 +995,8 @@ line "Features:", like this::
# @feature: Description text
A tagged section begins with a paragraph that starts with one of the
-following words: "Since:", "Example:"/"Examples:", "Returns:",
-"Errors:", "TODO:". It ends with the start of a new section.
+following words: "Since:", "Returns:", "Errors:", "TODO:". It ends with
+the start of a new section.
The second and subsequent lines of tagged sections must be indented
like this::
@@ -1020,13 +1020,53 @@ detailing a relevant error condition. For example::
A "Since: x.y.z" tagged section lists the release that introduced the
definition.
-An "Example" or "Examples" section is rendered entirely
-as literal fixed-width text. "TODO" sections are not rendered at all
-(they are for developers, not users of QMP). In other sections, the
-text is formatted, and rST markup can be used.
+"TODO" sections are not rendered at all (they are for developers, not
+users of QMP). In other sections, the text is formatted, and rST markup
+can be used.
+
+QMP Examples can be added by using the ``.. qmp-example::``
+directive. In its simplest form, this can be used to contain a single
+QMP code block which accepts standard JSON syntax with additional server
+directionality indicators (``->`` and ``<-``), and elisions (``...``).
+
+Optionally, a plaintext title may be provided by using the ``:title:``
+directive option. If the title is omitted, the example title will
+default to "Example:".
+
+A simple QMP example::
+
+ # .. qmp-example::
+ #:title: Using query-block
+ #
+ #-> { "execute": "query-block" }
+ #<- { ... }
+
+More complex or multi-step examples where exposition is needed before or
+between QMP code blocks can be created by using the ``:annotated:``
+directive option. When using this option, nested QMP code blocks must be
+entered explicitly with rST's ``::`` syntax.
+
+Highlighting in non-QMP languages can be accomplished by using the
+``.. code-bloc
[PATCH 6/8] qapi: convert "Example" sections with titles
When an Example section has a brief explanation, convert it to a
qmp-example:: section using the :title: option.
Rule of thumb: If the title can fit on a single line and requires no rST
markup, it's a good candidate for using the :title: option of
qmp-example.
In this patch, trailing punctuation is removed from the title section
for consistent headline aesthetics. In just one case, specifics of the
example are removed to make the title read better.
See commit-4: "docs/qapidoc: create qmp-example directive", for a
detailed explanation of this custom directive syntax.
See commit+2: "qapi: remove "Example" doc section" for a detailed
explanation of why.
Signed-off-by: John Snow
---
qapi/block-core.json | 24
qapi/block.json | 13 ++---
qapi/migration.json | 25 ++---
qapi/qom.json| 8
qapi/ui.json | 11 ++-
qapi/virtio.json | 19 ++-
6 files changed, 52 insertions(+), 48 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4e0f0395146..a371e3464d2 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -5885,9 +5885,8 @@
#
# Since: 2.7
#
-# Examples:
-#
-# 1. Add a new node to a quorum
+# .. qmp-example::
+#:title: Add a new node to a quorum
#
# -> { "execute": "blockdev-add",
# "arguments": {
@@ -5901,7 +5900,8 @@
# "node": "new_node" } }
# <- { "return": {} }
#
-# 2. Delete a quorum's node
+# .. qmp-example::
+#:title: Delete a quorum's node
#
# -> { "execute": "x-blockdev-change",
# "arguments": { "parent": "disk1",
@@ -5937,16 +5937,16 @@
#
# Since: 2.12
#
-# Examples:
-#
-# 1. Move a node into an IOThread
+# .. qmp-example::
+#:title: Move a node into an IOThread
#
# -> { "execute": "x-blockdev-set-iothread",
# "arguments": { "node-name": "disk1",
# "iothread": "iothread0" } }
# <- { "return": {} }
#
-# 2. Move a node into the main loop
+# .. qmp-example::
+#:title: Move a node into the main loop
#
# -> { "execute": "x-blockdev-set-iothread",
# "arguments": { "node-name": "disk1",
@@ -6022,16 +6022,16 @@
#
# Since: 2.0
#
-# Examples:
-#
-# 1. Read operation
+# .. qmp-example::
+#:title: Read operation
#
# <- { "event": "QUORUM_REPORT_BAD",
# "data": { "node-name": "node0", "sector-num": 345435,
"sectors-count": 5,
#"type": "read" },
# "timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
#
-# 2. Flush operation
+# .. qmp-example::
+#:title: Flush operation
#
# <- { "event": "QUORUM_REPORT_BAD",
# "data": { "node-name": "node0", "sector-num": 0, "sectors-count":
2097120,
diff --git a/qapi/block.json b/qapi/block.json
index c8e52bc2d29..5ddd061e964 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -342,9 +342,8 @@
#
# Since: 2.5
#
-# Examples:
-#
-# 1. Change a removable medium
+# .. qmp-example::
+#:title: Change a removable medium
#
# -> { "execute": "blockdev-change-medium",
# "arguments": { "id": "ide0-1-0",
@@ -352,7 +351,8 @@
# "format": "raw" } }
# <- { "return": {} }
#
-# 2. Load a read-only medium into a writable drive
+# .. qmp-example::
+#:title: Load a read-only medium into a writable drive
#
# -> { "execute": "blockdev-change-medium",
# "arguments": { "id": "floppyA",
@@ -577,9 +577,8 @@
# "boundaries-write": [1000, 5000] } }
# <- { "return": {} }
#
-# Example:
-#
-# Remove all latency histograms:
+# .. qmp-example::
+#:title: Remove all latency histograms
#
# -> { "execute": "block-latency-histogram-set",
# "arguments": { "id": "drive0" } }
diff --git a/qapi/migration.json b/qapi/migration.json
index a4391ea7e6f..37ce8afa380 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -287,14 +287,14 @@
#
# Since: 0.14
#
-# Examples:
-#
-# 1. Before the first migration
+# .. qmp-example::
+#:title: Before the first migration
#
# -> { "execute": "query-migrate" }
# <- { "return": {} }
#
-# 2. Migration is done and has succeeded
+# .. qmp-example::
+#:title: Migration is done and has succeeded
#
# -> { "execute": "query-migrate" }
# <- { "return": {
@@ -314,12 +314,14 @@
# }
#}
#
-# 3. Migration is done and has failed
+# .. qmp-example::
+#:title: Migration is done and has failed
#
# -> { "execute": "query-migrate" }
# <- { "return": { "status": "failed" } }
#
-# 4. Migration is being performed:
+# .. qmp-example::
+#:title: Migration is being performed
#
# -> { "execute": "query-migrate" }
# <- {
@@ -340,7 +342,8 @@
# }
#}
#
-# 5. Migration is being performed and XBZRLE is active:
+# .. qmp-example:
[PATCH 4/8] docs/sphinx: add CSS styling for qmp-example directive
From: Harmonie Snow
Add CSS styling for qmp-example directives to increase readability and
consistently style all example blocks.
Signed-off-by: Harmonie Snow
Signed-off-by: John Snow
---
docs/sphinx-static/theme_overrides.css | 49 ++
1 file changed, 49 insertions(+)
diff --git a/docs/sphinx-static/theme_overrides.css
b/docs/sphinx-static/theme_overrides.css
index c70ef951286..965ecac54fd 100644
--- a/docs/sphinx-static/theme_overrides.css
+++ b/docs/sphinx-static/theme_overrides.css
@@ -87,6 +87,55 @@ div[class^="highlight"] pre {
padding-bottom: 1px;
}
+/* qmp-example directive styling */
+
+.rst-content .admonition-example {
+/* do not apply the standard admonition background */
+background-color: transparent;
+border: solid #ffd2ed 1px;
+}
+
+.rst-content .admonition-example > .admonition-title:before {
+content: "▷";
+}
+
+.rst-content .admonition-example > .admonition-title {
+background-color: #5980a6;
+}
+
+.rst-content .admonition-example > div[class^="highlight"] {
+/* make code boxes take up the full width of the admonition w/o margin */
+margin-left: -12px;
+margin-right: -12px;
+
+border-top: 1px solid #ffd2ed;
+border-bottom: 1px solid #ffd2ed;
+border-left: 0px;
+border-right: 0px;
+}
+
+.rst-content .admonition-example > div[class^="highlight"]:nth-child(2) {
+/* If a code box is the second element in an example admonition,
+ * it is the first child after the title. let it sit flush against
+ * the title. */
+margin-top: -12px;
+border-top: 0px;
+}
+
+.rst-content .admonition-example > div[class^="highlight"]:last-child {
+/* If a code box is the final element in an example admonition, don't
+ * render margin below it; let it sit flush with the end of the
+ * admonition box */
+margin-bottom: -12px;
+border-bottom: 0px;
+}
+
+.rst-content .admonition-example .highlight {
+background-color: #fffafd;
+}
+
+/* end qmp-example styling */
+
@media screen {
/* content column
--
2.45.0
[PATCH 7/8] qapi: convert "Example" sections with longer prose
These examples require longer explanations or have explanations that
require markup to look reasonable when rendered and so use the longer
form of the ".. qmp-example::" directive.
By using the :annotated: option, the content in the example block is
assumed *not* to be a code block literal and is instead parsed as normal
rST - with the exception that any code literal blocks after `::` will
assumed to be a QMP code literal block.
Note: There's one title-less conversion in this patch that comes along
for the ride because it's part of a larger "Examples" block that was
better to convert all at once.
See commit-5: "docs/qapidoc: create qmp-example directive", for a
detailed explanation of this custom directive syntax.
See commit+1: "qapi: remove "Example" doc section" for a detailed
explanation of why.
Signed-off-by: John Snow
---
qapi/block.json | 26 --
qapi/machine.json | 30 --
qapi/migration.json | 7 +--
qapi/virtio.json| 24 ++--
4 files changed, 59 insertions(+), 28 deletions(-)
diff --git a/qapi/block.json b/qapi/block.json
index 5ddd061e964..d95e9fd8140 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -545,31 +545,37 @@
#
# Since: 4.0
#
-# Example:
+# .. qmp-example::
+#:annotated:
#
-# Set new histograms for all io types with intervals
-# [0, 10), [10, 50), [50, 100), [100, +inf):
+#Set new histograms for all io types with intervals
+#[0, 10), [10, 50), [50, 100), [100, +inf)::
#
# -> { "execute": "block-latency-histogram-set",
# "arguments": { "id": "drive0",
# "boundaries": [10, 50, 100] } }
# <- { "return": {} }
#
-# Example:
+# .. qmp-example::
+#:annotated:
#
-# Set new histogram only for write, other histograms will remain
-# not changed (or not created):
+#Set new histogram only for write, other histograms will remain
+#not changed (or not created)::
#
# -> { "execute": "block-latency-histogram-set",
# "arguments": { "id": "drive0",
# "boundaries-write": [10, 50, 100] } }
# <- { "return": {} }
#
-# Example:
+# .. qmp-example::
+#:annotated:
#
-# Set new histograms with the following intervals:
-# read, flush: [0, 10), [10, 50), [50, 100), [100, +inf)
-# write: [0, 1000), [1000, 5000), [5000, +inf)
+#Set new histograms with the following intervals:
+#
+#- read, flush: [0, 10), [10, 50), [50, 100), [100, +inf)
+#- write: [0, 1000), [1000, 5000), [5000, +inf)
+#
+#::
#
# -> { "execute": "block-latency-histogram-set",
# "arguments": { "id": "drive0",
diff --git a/qapi/machine.json b/qapi/machine.json
index 83f60b319c7..0a5ffe652b7 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1047,10 +1047,11 @@
#
# Since: 2.7
#
-# Examples:
+# .. qmp-example::
+#:annotated:
#
-# For pseries machine type started with -smp 2,cores=2,maxcpus=4
-# -cpu POWER8:
+#For pseries machine type started with
+#``-smp 2,cores=2,maxcpus=4 -cpu POWER8``::
#
# -> { "execute": "query-hotpluggable-cpus" }
# <- {"return": [
@@ -1060,7 +1061,10 @@
#"vcpus-count": 1, "qom-path": "/machine/unattached/device[0]"}
#]}
#
-# For pc machine type started with -smp 1,maxcpus=2:
+# .. qmp-example::
+#:annotated:
+#
+#For pc machine type started with ``-smp 1,maxcpus=2``::
#
# -> { "execute": "query-hotpluggable-cpus" }
# <- {"return": [
@@ -1075,8 +1079,11 @@
# }
#]}
#
-# For s390x-virtio-ccw machine type started with -smp 1,maxcpus=2
-# -cpu qemu (Since: 2.11):
+# .. qmp-example::
+#:annotated:
+#
+#For s390x-virtio-ccw machine type started with
+#``-smp 1,maxcpus=2 -cpu qemu`` (Since: 2.11)::
#
# -> { "execute": "query-hotpluggable-cpus" }
# <- {"return": [
@@ -1130,12 +1137,15 @@
#
# Since: 0.14
#
-# Example:
+# .. qmp-example::
+#:annotated:
#
-# -> { "execute": "balloon", "arguments": { "value": 536870912 } }
-# <- { "return": {} }
+#::
#
-# With a 2.5GiB guest this command inflated the ballon to 3GiB.
+# -> { "execute": "balloon", "arguments": { "value": 536870912 } }
+# <- { "return": {} }
+#
+#With a 2.5GiB guest this command inflated the ballon to 3GiB.
##
{ 'command': 'balloon', 'data': {'value': 'int'} }
diff --git a/qapi/migration.json b/qapi/migration.json
index 37ce8afa380..e208a86258a 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -2106,13 +2106,16 @@
#
# Since: 5.2
#
-# Example:
+# .. qmp-example::
#
# -> {"execute": "calc-dirty-rate", "arguments": {"calc-time": 1,
# "sample-pages": 512} }
# <- { "return": {} }
#
-# Measure dirty rate using dirty bitmap for 500 milliseconds:
+# .. qmp-example::
+#:annotated:
+#
+#Measure dirty rate usi
[PATCH 3/8] docs/qapidoc: add QMP highlighting to annotated qmp-example blocks
For any code literal blocks inside of a qmp-example directive, apply and
enforce the QMP lexer/highlighter to those blocks.
This way, you won't need to write:
```
.. qmp-example::
:annotated:
Blah blah
.. code-block:: QMP
-> { "lorem": "ipsum" }
```
But instead, simply:
```
.. qmp-example::
:annotated:
Blah blah::
-> { "lorem": "ipsum" }
```
Once the directive block is exited, whatever the previous default
highlight language was will be restored; localizing the forced QMP
lexing to exclusively this directive.
Note, if the default language is *already* QMP, this directive will not
generate and restore redundant highlight configuration nodes. We may
well decide that the default language ought to be QMP for any QAPI
reference pages, but this way the directive behaves consistently no
matter where it is used.
Signed-off-by: John Snow
---
docs/sphinx/qapidoc.py | 53 ++
1 file changed, 49 insertions(+), 4 deletions(-)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index b0f3917dc5b..fb2b23698a0 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -26,6 +26,7 @@
import os
import re
+import sys
import textwrap
from typing import List
@@ -37,6 +38,7 @@
from qapi.schema import QAPISchema
import sphinx
+from sphinx import addnodes
from sphinx.directives.code import CodeBlock
from sphinx.errors import ExtensionError
from sphinx.util.nodes import nested_parse_with_titles
@@ -574,10 +576,10 @@ class QMPExample(CodeBlock, NestedDirective):
Custom admonition for QMP code examples.
When the :annotated: option is present, the body of this directive
-is parsed as normal rST instead. Code blocks must be explicitly
-written by the user, but this allows for intermingling explanatory
-paragraphs with arbitrary rST syntax and code blocks for more
-involved examples.
+is parsed as normal rST, but with any '::' code blocks set to use
+the QMP lexer. Code blocks must be explicitly written by the user,
+but this allows for intermingling explanatory paragraphs with
+arbitrary rST syntax and code blocks for more involved examples.
When :annotated: is absent, the directive body is treated as a
simple standalone QMP code block literal.
@@ -591,6 +593,33 @@ class QMPExample(CodeBlock, NestedDirective):
"title": directives.unchanged,
}
+def _highlightlang(self) -> addnodes.highlightlang:
+"""Return the current highlightlang setting for the document"""
+node = None
+doc = self.state.document
+
+if hasattr(doc, "findall"):
+# docutils >= 0.18.1
+for node in doc.findall(addnodes.highlightlang):
+pass
+else:
+for elem in doc.traverse():
+if isinstance(elem, addnodes.highlightlang):
+node = elem
+
+if node:
+return node
+
+# No explicit directive found, use defaults
+node = addnodes.highlightlang(
+lang=self.env.config.highlight_language,
+force=False,
+# Yes, Sphinx uses this value to effectively disable line
+# numbers and not 0 or None or -1 or something. ¯\_(ツ)_/¯
+linenothreshold=sys.maxsize,
+)
+return node
+
def admonition_wrap(self, *content) -> List[nodes.Node]:
title = "Example:"
if "title" in self.options:
@@ -605,8 +634,24 @@ def admonition_wrap(self, *content) -> List[nodes.Node]:
return [admon]
def run_annotated(self) -> List[nodes.Node]:
+lang_node = self._highlightlang()
+
content_node: nodes.Element = nodes.section()
+
+# Configure QMP highlighting for "::" blocks, if needed
+if lang_node["lang"] != "QMP":
+content_node += addnodes.highlightlang(
+lang="QMP",
+force=False, # "True" ignores lexing errors
+linenothreshold=lang_node["linenothreshold"],
+)
+
self.do_parse(self.content, content_node)
+
+# Restore prior language highlighting, if needed
+if lang_node["lang"] != "QMP":
+content_node += addnodes.highlightlang(**lang_node.attributes)
+
return content_node.children
def run(self) -> List[nodes.Node]:
--
2.45.0
[PATCH 2/8] docs/qapidoc: create qmp-example directive
This is a directive that creates a syntactic sugar for creating
"Example" boxes very similar to the ones already used in the bitmaps.rst
document, please see e.g.
https://www.qemu.org/docs/master/interop/bitmaps.html#creation-block-dirty-bitmap-add
In its simplest form, when a custom title is not needed or wanted, and
the example body is *solely* a QMP example:
```
.. qmp-example::
{body}
```
is syntactic sugar for:
```
.. admonition:: Example:
.. code-block:: QMP
{body}
```
When a custom, plaintext title that describes the example is desired,
this form:
```
.. qmp-example::
:title: Defrobnification
{body}
```
Is syntactic sugar for:
```
.. admonition:: Example: Defrobnification
.. code-block:: QMP
{body}
```
Lastly, when Examples are multi-step processes that require non-QMP
exposition, have lengthy titles, or otherwise involve prose with rST
markup (lists, cross-references, etc), the most complex form:
```
.. qmp-example::
:annotated:
This example shows how to use `foo-command`::
{body}
For more information, please see `frobnozz`.
```
Is desugared to:
```
.. admonition:: Example:
This example shows how to use `foo-command`::
{body}
For more information, please see `frobnozz`.
```
Note that :annotated: and :title: options can be combined together, if
desired.
The primary benefit here being documentation source consistently using
the same directive for all forms of examples to ensure consistent visual
styling, and ensuring all relevant prose is visually grouped alongside
the code literal block.
Note that as of this commit, the code-block rST syntax "::" does not
apply QMP highlighting; you would need to use ".. code-block:: QMP". The
very next commit changes this behavior to assume all "::" code blocks
within this directive are QMP blocks.
Signed-off-by: John Snow
---
docs/sphinx/qapidoc.py | 55 ++
1 file changed, 55 insertions(+)
diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 43dd99e21e6..b0f3917dc5b 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -27,6 +27,7 @@
import os
import re
import textwrap
+from typing import List
from docutils import nodes
from docutils.parsers.rst import Directive, directives
@@ -36,6 +37,7 @@
from qapi.schema import QAPISchema
import sphinx
+from sphinx.directives.code import CodeBlock
from sphinx.errors import ExtensionError
from sphinx.util.nodes import nested_parse_with_titles
@@ -567,10 +569,63 @@ def run(self):
raise ExtensionError(str(err)) from err
+class QMPExample(CodeBlock, NestedDirective):
+"""
+Custom admonition for QMP code examples.
+
+When the :annotated: option is present, the body of this directive
+is parsed as normal rST instead. Code blocks must be explicitly
+written by the user, but this allows for intermingling explanatory
+paragraphs with arbitrary rST syntax and code blocks for more
+involved examples.
+
+When :annotated: is absent, the directive body is treated as a
+simple standalone QMP code block literal.
+"""
+
+required_argument = 0
+optional_arguments = 0
+has_content = True
+option_spec = {
+"annotated": directives.flag,
+"title": directives.unchanged,
+}
+
+def admonition_wrap(self, *content) -> List[nodes.Node]:
+title = "Example:"
+if "title" in self.options:
+title = f"{title} {self.options['title']}"
+
+admon = nodes.admonition(
+"",
+nodes.title("", title),
+*content,
+classes=["admonition", "admonition-example"],
+)
+return [admon]
+
+def run_annotated(self) -> List[nodes.Node]:
+content_node: nodes.Element = nodes.section()
+self.do_parse(self.content, content_node)
+return content_node.children
+
+def run(self) -> List[nodes.Node]:
+annotated = "annotated" in self.options
+
+if annotated:
+content_nodes = self.run_annotated()
+else:
+self.arguments = ["QMP"]
+content_nodes = super().run()
+
+return self.admonition_wrap(*content_nodes)
+
+
def setup(app):
"""Register qapi-doc directive with Sphinx"""
app.add_config_value("qapidoc_srctree", None, "env")
app.add_directive("qapi-doc", QAPIDocDirective)
+app.add_directive("qmp-example", QMPExample)
return {
"version": __version__,
--
2.45.0
[PATCH 0/8] qapi: convert example sections to qmp-example rST directives
GitLab: https://gitlab.com/jsnow/qemu/-/pipelines/1359714660 This patchset focuses on converting example sections to rST directives using a new `.. qmp-example::` directive. It is based on what I *assume* will be Markus' next pull request that covers note conversion. Pull these patches from GitLab directly if that's too annoying: https://gitlab.com/jsnow/qemu/-/commits/sphinx-domain-prereqs-examples It is also annoyingly the case that both Markus' next pull request and this series conflicts with a separate series I sent out, "docs/python: bump minimum Sphinx version" - so it's extremely likely I'll need to rebase and respin this series depending on what goes in and in what order. Ah well... Changes since this was split out from the prior series: - Harmonie updated the CSS for the example block section. I think it's really tidy now! Thanks Harmonie! - Dependence on SphinxDirective was removed, but it will likely re-appear in the next series anyway. - qapi-code-gen.rst was updated with a section on how to write examples. - Various minor tweaks to comments, commit messages, docs, etc. Harmonie Snow (1): docs/sphinx: add CSS styling for qmp-example directive John Snow (7): docs/qapidoc: factor out do_parse() docs/qapidoc: create qmp-example directive docs/qapidoc: add QMP highlighting to annotated qmp-example blocks qapi: convert "Example" sections without titles qapi: convert "Example" sections with titles qapi: convert "Example" sections with longer prose qapi: remove "Example" doc section docs/devel/qapi-code-gen.rst | 58 +++-- docs/sphinx-static/theme_overrides.css | 49 docs/sphinx/qapidoc.py | 156 + qapi/acpi.json | 4 +- qapi/block-core.json | 88 +++--- qapi/block.json| 57 + qapi/char.json | 24 ++-- qapi/control.json | 8 +- qapi/dump.json | 8 +- qapi/machine-target.json | 2 +- qapi/machine.json | 68 ++- qapi/migration.json| 90 +++--- qapi/misc-target.json | 22 ++-- qapi/misc.json | 32 ++--- qapi/net.json | 20 ++-- qapi/pci.json | 2 +- qapi/qdev.json | 10 +- qapi/qom.json | 16 +-- qapi/replay.json | 8 +- qapi/rocker.json | 8 +- qapi/run-state.json| 30 ++--- qapi/tpm.json | 6 +- qapi/trace.json| 4 +- qapi/transaction.json | 2 +- qapi/ui.json | 45 +++ qapi/vfio.json | 2 +- qapi/virtio.json | 45 --- qapi/yank.json | 4 +- scripts/qapi/parser.py | 10 +- tests/qapi-schema/doc-good.json| 19 +-- tests/qapi-schema/doc-good.out | 26 +++-- tests/qapi-schema/doc-good.txt | 23 ++-- 32 files changed, 605 insertions(+), 341 deletions(-) -- 2.45.0
[PATCH 1/8] docs/qapidoc: factor out do_parse()
Factor out the compatibility parser helper into a base class, so it can be shared by other directives. Signed-off-by: John Snow --- docs/sphinx/qapidoc.py | 64 +++--- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index efcd84656fa..43dd99e21e6 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -494,7 +494,41 @@ def visit_module(self, name): super().visit_module(name) -class QAPIDocDirective(Directive): +class NestedDirective(Directive): +def run(self): +raise NotImplementedError + +def do_parse(self, rstlist, node): +""" +Parse rST source lines and add them to the specified node + +Take the list of rST source lines rstlist, parse them as +rST, and add the resulting docutils nodes as children of node. +The nodes are parsed in a way that allows them to include +subheadings (titles) without confusing the rendering of +anything else. +""" +# This is from kerneldoc.py -- it works around an API change in +# Sphinx between 1.6 and 1.7. Unlike kerneldoc.py, we use +# sphinx.util.nodes.nested_parse_with_titles() rather than the +# plain self.state.nested_parse(), and so we can drop the saving +# of title_styles and section_level that kerneldoc.py does, +# because nested_parse_with_titles() does that for us. +if USE_SSI: +with switch_source_input(self.state, rstlist): +nested_parse_with_titles(self.state, rstlist, node) +else: +save = self.state.memo.reporter +self.state.memo.reporter = AutodocReporter( +rstlist, self.state.memo.reporter +) +try: +nested_parse_with_titles(self.state, rstlist, node) +finally: +self.state.memo.reporter = save + + +class QAPIDocDirective(NestedDirective): """Extract documentation from the specified QAPI .json file""" required_argument = 1 @@ -532,34 +566,6 @@ def run(self): # so they are displayed nicely to the user raise ExtensionError(str(err)) from err -def do_parse(self, rstlist, node): -"""Parse rST source lines and add them to the specified node - -Take the list of rST source lines rstlist, parse them as -rST, and add the resulting docutils nodes as children of node. -The nodes are parsed in a way that allows them to include -subheadings (titles) without confusing the rendering of -anything else. -""" -# This is from kerneldoc.py -- it works around an API change in -# Sphinx between 1.6 and 1.7. Unlike kerneldoc.py, we use -# sphinx.util.nodes.nested_parse_with_titles() rather than the -# plain self.state.nested_parse(), and so we can drop the saving -# of title_styles and section_level that kerneldoc.py does, -# because nested_parse_with_titles() does that for us. -if USE_SSI: -with switch_source_input(self.state, rstlist): -nested_parse_with_titles(self.state, rstlist, node) -else: -save = self.state.memo.reporter -self.state.memo.reporter = AutodocReporter( -rstlist, self.state.memo.reporter -) -try: -nested_parse_with_titles(self.state, rstlist, node) -finally: -self.state.memo.reporter = save - def setup(app): """Register qapi-doc directive with Sphinx""" -- 2.45.0
Re: [PATCH 3/4] iotests: Change imports for Python 3.13
On Tue, Jul 2, 2024, 1:51 PM Nir Soffer wrote: > > On 2 Jul 2024, at 17:44, John Snow wrote: > > > > On Tue, Jul 2, 2024 at 7:52 AM Nir Soffer wrote: > >> On Thu, Jun 27, 2024 at 2:23 AM John Snow wrote: >> > >> > Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to >> > make it the default system interpreter for Fedora 41. >> > >> > They moved our cheese for where ContextManager lives; add a conditional >> > to locate it while we support both pre-3.9 and 3.13+. >> > >> > Signed-off-by: John Snow >> > --- >> > tests/qemu-iotests/testenv.py| 7 ++- >> > tests/qemu-iotests/testrunner.py | 9 ++--- >> > 2 files changed, 12 insertions(+), 4 deletions(-) >> > >> > diff --git a/tests/qemu-iotests/testenv.py >> b/tests/qemu-iotests/testenv.py >> > index 588f30a4f14..96d69e56963 100644 >> > --- a/tests/qemu-iotests/testenv.py >> > +++ b/tests/qemu-iotests/testenv.py >> > @@ -25,7 +25,12 @@ >> > import random >> > import subprocess >> > import glob >> > -from typing import List, Dict, Any, Optional, ContextManager >> > +from typing import List, Dict, Any, Optional >> > + >> > +if sys.version_info >= (3, 9): >> > +from contextlib import AbstractContextManager as ContextManager >> > +else: >> > +from typing import ContextManager >> >> It can be cleaner to add a compat module hiding the details so the >> entire project >> can have a single instance of this. Other code will just use: >> >> from compat import ContextManager >> > > If there were more than two uses, I'd consider it. As it stands, a > compat.py module with just one import conditional in it doesn't seem worth > the hassle. Are there more cases of compatibility goop inside iotests that > need to be factored out to make it worth it? > > > I don’t about other. For me even one instance is ugly enough :-) > I was going to add it to qemu/utils, but then I remembered the testenv/testrunner script here needs to operate without external dependencies because part of the function of these modules is to *locate* those dependencies. Ehh. I'm going to say that repeating the import scaffolding in just two places is fine enough for now and really not worth adding a compat.py for *just* this. Let's just get the tests green. --js
Re: [PATCH v3] virtio: Implement Virtio Backend for SD/MMC in QEMU
On Wed, Jul 03, 2024 at 10:55:17PM +0300, Mikhail Krasheninnikov wrote: > > Hello, Alex! > > No, there's no patch to the VirtIO specification yet. This is > proof-of-concept solution since I'm not sure that I did everything > correct with the design (and as folks' reviews show, for a good reason). > As soon as most obvious issues would be out of the way, I think I'll > submit a patch. Mikhail, if you want people to review your patches but not merge them yet, pls use an RFC tag in the subject to avoid confusion. Thanks, -- MST
Re: [PATCH v3] virtio: Implement Virtio Backend for SD/MMC in QEMU
Hello, Alex! No, there's no patch to the VirtIO specification yet. This is proof-of-concept solution since I'm not sure that I did everything correct with the design (and as folks' reviews show, for a good reason). As soon as most obvious issues would be out of the way, I think I'll submit a patch.
Re: [PULL 0/4] Block layer patches (CVE-2024-4467)
On 7/2/24 09:39, Kevin Wolf wrote: The following changes since commit c80a339587fe4148292c260716482dd2f86d4476: Merge tag 'pull-target-arm-20240701' ofhttps://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-07-01 10:41:45 -0700) are available in the Git repository at: https://repo.or.cz/qemu/kevin.git tags/for-upstream for you to fetch changes up to 7ead946998610657d38d1a505d5f25300d4ca613: block: Parse filenames only when explicitly requested (2024-07-02 18:12:30 +0200) Block layer patches (CVE-2024-4467) - Don't open qcow2 data files in 'qemu-img info' - Disallow protocol prefixes for qcow2 data files, VMDK extent files and other child nodes that are neither 'file' nor 'backing' Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/9.1 as appropriate. r~
Re: [PATCH v3] virtio: Implement Virtio Backend for SD/MMC in QEMU
Mikhail Krasheninnikov writes: > From: Mi > > Add a Virtio backend for SD/MMC devices. Confirmed interoperability with > Linux. > > Linux patch link: > https://lore.kernel.org/linux-mmc/20240701120642.30001-1-krashmi...@gmail.com/ Is there a corresponding patch to the VirtIO specification? -- Alex Bennée Virtualisation Tech Lead @ Linaro
[PATCH v3] virtio: Implement Virtio Backend for SD/MMC in QEMU
From: Mi
Add a Virtio backend for SD/MMC devices. Confirmed interoperability with
Linux.
Linux patch link:
https://lore.kernel.org/linux-mmc/20240701120642.30001-1-krashmi...@gmail.com/
Signed-off-by: Mikhail Krasheninnikov
CC: Matwey Kornilov
CC: qemu-block@nongnu.org
CC: Michael S. Tsirkin
CC: Kevin Wolf
CC: Stefan Hajnoczi
---
Thanks for the feedback!
Changes from v2:
- Renamed virtio-mmc to virtio-sdhci
- Added a link to linux patch with corresponding driver in commit message
- Moved variable declaration to function prologue
- Removed duplicate defines
- Changed #pragma once
Michael, I couldn't find any simular structs to those in virtio-sdhci.c in Linux
uapi, I'll be very grateful if you could point them out to me!
hw/block/Kconfig| 6 +
hw/block/meson.build| 1 +
hw/block/virtio-sdhci.c | 169
hw/virtio/meson.build | 1 +
hw/virtio/virtio-sdhci-pci.c| 86 ++
hw/virtio/virtio.c | 3 +-
include/hw/virtio/virtio-sdhci.h| 22 +++
include/standard-headers/linux/virtio_ids.h | 1 +
8 files changed, 288 insertions(+), 1 deletion(-)
create mode 100644 hw/block/virtio-sdhci.c
create mode 100644 hw/virtio/virtio-sdhci-pci.c
create mode 100644 include/hw/virtio/virtio-sdhci.h
diff --git a/hw/block/Kconfig b/hw/block/Kconfig
index 9e8f28f982..5b5363da01 100644
--- a/hw/block/Kconfig
+++ b/hw/block/Kconfig
@@ -44,3 +44,9 @@ config VHOST_USER_BLK
config SWIM
bool
+
+config VIRTIO_SDHCI
+bool
+default y
+select SD
+depends on VIRTIO
diff --git a/hw/block/meson.build b/hw/block/meson.build
index 8aa4dc3893..82356c264e 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -19,5 +19,6 @@ system_ss.add(when: 'CONFIG_TC58128', if_true:
files('tc58128.c'))
specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c',
'virtio-blk-common.c'))
specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true:
files('vhost-user-blk.c', 'virtio-blk-common.c'))
+specific_ss.add(when: 'CONFIG_VIRTIO_SDHCI', if_true: files('virtio-sdhci.c'))
subdir('dataplane')
diff --git a/hw/block/virtio-sdhci.c b/hw/block/virtio-sdhci.c
new file mode 100644
index 00..8b7105229f
--- /dev/null
+++ b/hw/block/virtio-sdhci.c
@@ -0,0 +1,169 @@
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-sdhci.h"
+#include "qemu/typedefs.h"
+#include "sysemu/blockdev.h"
+#include "standard-headers/linux/virtio_ids.h"
+
+typedef struct sd_req {
+uint32_t opcode;
+uint32_t arg;
+} sd_req;
+
+typedef struct virtio_sdhci_req {
+uint8_t flags;
+
+#define VIRTIO_SDHCI_REQUEST_DATA BIT(1)
+#define VIRTIO_SDHCI_REQUEST_WRITE BIT(2)
+#define VIRTIO_SDHCI_REQUEST_STOP BIT(3)
+#define VIRTIO_SDHCI_REQUEST_SBC BIT(4)
+
+sd_req request;
+
+uint8_t buf[4096];
+size_t buf_len;
+
+sd_req stop_req;
+sd_req sbc_req;
+} virtio_sdhci_req;
+
+typedef struct virtio_sdhci_resp {
+uint32_t response[4];
+int resp_len;
+uint8_t buf[4096];
+} virtio_sdhci_resp;
+
+static void send_command(SDBus *sdbus, sd_req *mmc_request, uint8_t *response,
+ virtio_sdhci_resp *virtio_resp)
+{
+SDRequest sdreq;
+int resp_len;
+
+sdreq.cmd = (uint8_t)mmc_request->opcode;
+sdreq.arg = mmc_request->arg;
+resp_len = sdbus_do_command(sdbus, &sdreq, response);
+virtio_resp->resp_len = resp_len;
+
+for (int i = 0; i < resp_len / sizeof(uint32_t); i++) {
+virtio_resp->response[i] = ldl_be_p(&virtio_resp->response[i]);
+}
+}
+
+static void send_command_without_response(SDBus *sdbus, sd_req *mmc_request)
+{
+SDRequest sdreq;
+uint8_t response[4];
+
+sdreq.cmd = (uint8_t)mmc_request->opcode;
+sdreq.arg = mmc_request->arg;
+sdbus_do_command(sdbus, &sdreq, response);
+}
+
+static void handle_mmc_request(VirtIODevice *vdev, virtio_sdhci_req
*virtio_req,
+ virtio_sdhci_resp *virtio_resp)
+{
+VirtIOSDHCI *vsd = VIRTIO_SDHCI(vdev);
+SDBus *sdbus = &vsd->sdbus;
+
+if (virtio_req->flags & VIRTIO_SDHCI_REQUEST_SBC) {
+send_command_without_response(sdbus, &virtio_req->sbc_req);
+}
+
+send_command(sdbus, &virtio_req->request,
+(uint8_t *)virtio_resp->response, virtio_resp);
+
+if (virtio_req->flags & VIRTIO_SDHCI_REQUEST_DATA) {
+if (virtio_req->flags & VIRTIO_SDHCI_REQUEST_WRITE) {
+sdbus_write_data(sdbus, virtio_req->buf, virtio_req->buf_len);
+} else {
+sdbus_read_data(sdbus, virtio_resp->buf, virtio_req->buf_len);
+}
+}
+
+if (virtio_req->flags & VIRTIO_SDHCI_REQUEST_STOP) {
+send_command_without_response(sdbus, &virtio_req->stop_req);
+}
+}
+
+static void handle_request(VirtIODevice *vdev, VirtQueue *vq)
+{
+VirtQueueElement *elem;
+
Re: [PATCH v10 11/12] hw/pci: Convert rom_bar into OnOffAuto
On Wed, 3 Jul 2024 13:00:21 +0200 (CEST)
BALATON Zoltan wrote:
> On Wed, 3 Jul 2024, Michael S. Tsirkin wrote:
> > On Wed, Jul 03, 2024 at 04:15:23AM +0200, BALATON Zoltan wrote:
> >> On Tue, 2 Jul 2024, Michael S. Tsirkin wrote:
> >>> On Thu, Jun 27, 2024 at 03:08:00PM +0900, Akihiko Odaki wrote:
> rom_bar is tristate but was defined as uint32_t so convert it into
> OnOffAuto.
>
> Signed-off-by: Akihiko Odaki
> >>>
> >>> Commit log should explain why this is an improvement,
> >>> not just what's done.
> >>>
> >>>
> diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
> index e17bb50789ad..35c6c8e28493 100644
> --- a/docs/igd-assign.txt
> +++ b/docs/igd-assign.txt
> @@ -35,7 +35,7 @@ IGD has two different modes for assignment using
> vfio-pci:
> ISA/LPC bridge device (vfio-pci-igd-lpc-bridge) on the root bus at
> PCI address 1f.0.
> * The IGD device must have a VGA ROM, either provided via the
> romfile
> - option or loaded automatically through vfio (standard). rombar=0
> + option or loaded automatically through vfio (standard).
> rombar=off
> will disable legacy mode support.
> * Hotplug of the IGD device is not supported.
> * The IGD device must be a SandyBridge or newer model device.
> >>>
> >>> ...
> >>>
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index 39dae72497e0..0e920ed0691a 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -33,7 +33,7 @@
> * execution as noticed with the BCM 57810 card for lack of a
> * more better way to handle such issues.
> * The user can still override by specifying a romfile or
> - * rombar=1.
> + * rombar=on.
> * Please see https://bugs.launchpad.net/qemu/+bug/1284874
> * for an analysis of the 57810 card hang. When adding
> * a new vendor id/device id combination below, please also add
> >>>
> >>>
> >>> So we are apparently breaking a bunch of users who followed
> >>> documentation to the dot. Why is this a good idea?
> >>
> >> On/off is clearer than 1/0. But isn't 1/0 a synonym for on/off so previous
> >> command lines would still work?
> >>
> >> Regards,
> >> BALATON Zoltan
> >
> > I see nothing in code that would make it so:
> >
> >
> > const QEnumLookup OnOffAuto_lookup = {
> >.array = (const char *const[]) {
> >[ON_OFF_AUTO_AUTO] = "auto",
> >[ON_OFF_AUTO_ON] = "on",
> >[ON_OFF_AUTO_OFF] = "off",
> >},
> >.size = ON_OFF_AUTO__MAX
> > };
> >
> > I also tried with an existing property:
> >
> > $ ./qemu-system-x86_64 -device intel-hda,msi=0
> > qemu-system-x86_64: -device intel-hda,msi=0: Parameter 'msi' does not
> > accept value '0'
>
> Then it was probably bit properties that also accept 0/1, on/off,
> true/false. Maybe similar aliases could be added to on/off/auto?
>
> In any case when I first saw rombar I thought it would set the BAR of the
> ROM so wondered why it's 1 and not 5 or 6 or an offset. So on/off is
> clearer in this case.
There's only one PCI spec defined offset for the ROM BAR. Yes, the
option could be more clear but relocating the ROM to a different
regular BAR offset is invalid. Thanks,
Alex
Re: [PATCH v46 5/5] hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD
On 7/3/24 4:07 PM, Philippe Mathieu-Daudé wrote:
On 3/7/24 16:02, Cédric Le Goater wrote:
On 7/3/24 3:43 PM, Philippe Mathieu-Daudé wrote:
In order to keep eMMC model simpler to maintain,
extract common properties and the common code from
class_init to the (internal) TYPE_SDMMC_COMMON.
Update the corresponding QOM cast macros.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sdmmc-internal.h | 3 +++
hw/sd/core.c | 29
hw/sd/sd.c | 50 --
3 files changed, 52 insertions(+), 30 deletions(-)
@@ -2508,14 +2521,19 @@ static void sd_spi_class_init(ObjectClass *klass, void
*data)
static const TypeInfo sd_types[] = {
{
- .name = TYPE_SD_CARD,
+ .name = TYPE_SDMMC_COMMON,
.parent = TYPE_DEVICE,
.instance_size = sizeof(SDState),
.class_size = sizeof(SDCardClass),
- .class_init = sd_class_init,
+ .class_init = sdmmc_common_class_init,
.instance_init = sd_instance_init,
.instance_finalize = sd_instance_finalize,
},
Shouldn't it be an abstract class ?
Ah yes, safer. Squashing:
-- >8 --
@@ -2513,6 +2513,7 @@ static const TypeInfo sd_types[] = {
{
.name = TYPE_SDMMC_COMMON,
.parent = TYPE_DEVICE,
+ .abstract = true,
.instance_size = sizeof(SDState),
.class_size = sizeof(SDCardClass),
.class_init = sdmmc_common_class_init,
with that,
Reviewed-by: Cédric Le Goater
Thanks,
C.
Re: [PATCH v46 5/5] hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD
On 3/7/24 16:02, Cédric Le Goater wrote:
On 7/3/24 3:43 PM, Philippe Mathieu-Daudé wrote:
In order to keep eMMC model simpler to maintain,
extract common properties and the common code from
class_init to the (internal) TYPE_SDMMC_COMMON.
Update the corresponding QOM cast macros.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sdmmc-internal.h | 3 +++
hw/sd/core.c | 29
hw/sd/sd.c | 50 --
3 files changed, 52 insertions(+), 30 deletions(-)
@@ -2508,14 +2521,19 @@ static void sd_spi_class_init(ObjectClass
*klass, void *data)
static const TypeInfo sd_types[] = {
{
- .name = TYPE_SD_CARD,
+ .name = TYPE_SDMMC_COMMON,
.parent = TYPE_DEVICE,
.instance_size = sizeof(SDState),
.class_size = sizeof(SDCardClass),
- .class_init = sd_class_init,
+ .class_init = sdmmc_common_class_init,
.instance_init = sd_instance_init,
.instance_finalize = sd_instance_finalize,
},
Shouldn't it be an abstract class ?
Ah yes, safer. Squashing:
-- >8 --
@@ -2513,6 +2513,7 @@ static const TypeInfo sd_types[] = {
{
.name = TYPE_SDMMC_COMMON,
.parent = TYPE_DEVICE,
+.abstract = true,
.instance_size = sizeof(SDState),
.class_size = sizeof(SDCardClass),
.class_init = sdmmc_common_class_init,
---
Thanks,
C.
+ {
+ .name = TYPE_SD_CARD,
+ .parent = TYPE_SDMMC_COMMON,
+ .class_init = sd_class_init,
+ },
{
.name = TYPE_SD_CARD_SPI,
.parent = TYPE_SD_CARD,
Re: [PATCH v46 5/5] hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD
On 7/3/24 3:43 PM, Philippe Mathieu-Daudé wrote:
In order to keep eMMC model simpler to maintain,
extract common properties and the common code from
class_init to the (internal) TYPE_SDMMC_COMMON.
Update the corresponding QOM cast macros.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sdmmc-internal.h | 3 +++
hw/sd/core.c | 29
hw/sd/sd.c | 50 --
3 files changed, 52 insertions(+), 30 deletions(-)
diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
index cc0b69e834..91eb5b6b2f 100644
--- a/hw/sd/sdmmc-internal.h
+++ b/hw/sd/sdmmc-internal.h
@@ -11,6 +11,9 @@
#ifndef SDMMC_INTERNAL_H
#define SDMMC_INTERNAL_H
+#define TYPE_SDMMC_COMMON "sdmmc-common"
+DECLARE_OBJ_CHECKERS(SDState, SDCardClass, SDMMC_COMMON, TYPE_SDMMC_COMMON)
+
/*
* EXT_CSD Modes segment
*
diff --git a/hw/sd/core.c b/hw/sd/core.c
index 52d5d90045..4b30218b52 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -24,6 +24,7 @@
#include "hw/sd/sd.h"
#include "qemu/module.h"
#include "qapi/error.h"
+#include "sdmmc-internal.h"
#include "trace.h"
static inline const char *sdbus_name(SDBus *sdbus)
@@ -39,7 +40,7 @@ static SDState *get_card(SDBus *sdbus)
if (!kid) {
return NULL;
}
-return SD_CARD(kid->child);
+return SDMMC_COMMON(kid->child);
}
uint8_t sdbus_get_dat_lines(SDBus *sdbus)
@@ -48,7 +49,7 @@ uint8_t sdbus_get_dat_lines(SDBus *sdbus)
uint8_t dat_lines = 0b; /* 4 bit bus width */
if (slave) {
-SDCardClass *sc = SD_CARD_GET_CLASS(slave);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(slave);
if (sc->get_dat_lines) {
dat_lines = sc->get_dat_lines(slave);
@@ -65,7 +66,7 @@ bool sdbus_get_cmd_line(SDBus *sdbus)
bool cmd_line = true;
if (slave) {
-SDCardClass *sc = SD_CARD_GET_CLASS(slave);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(slave);
if (sc->get_cmd_line) {
cmd_line = sc->get_cmd_line(slave);
@@ -82,7 +83,7 @@ void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts)
trace_sdbus_set_voltage(sdbus_name(sdbus), millivolts);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
assert(sc->set_voltage);
sc->set_voltage(card, millivolts);
@@ -95,7 +96,7 @@ int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t
*response)
trace_sdbus_command(sdbus_name(sdbus), req->cmd, req->arg);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
return sc->do_command(card, req, response);
}
@@ -109,7 +110,7 @@ void sdbus_write_byte(SDBus *sdbus, uint8_t value)
trace_sdbus_write(sdbus_name(sdbus), value);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
sc->write_byte(card, value);
}
@@ -121,7 +122,7 @@ void sdbus_write_data(SDBus *sdbus, const void *buf, size_t
length)
const uint8_t *data = buf;
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
for (size_t i = 0; i < length; i++) {
trace_sdbus_write(sdbus_name(sdbus), data[i]);
@@ -136,7 +137,7 @@ uint8_t sdbus_read_byte(SDBus *sdbus)
uint8_t value = 0;
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
value = sc->read_byte(card);
}
@@ -151,7 +152,7 @@ void sdbus_read_data(SDBus *sdbus, void *buf, size_t length)
uint8_t *data = buf;
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
for (size_t i = 0; i < length; i++) {
data[i] = sc->read_byte(card);
@@ -165,7 +166,7 @@ bool sdbus_receive_ready(SDBus *sdbus)
SDState *card = get_card(sdbus);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
return sc->receive_ready(card);
}
@@ -178,7 +179,7 @@ bool sdbus_data_ready(SDBus *sdbus)
SDState *card = get_card(sdbus);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
return sc->data_ready(card);
}
@@ -191,7 +192,7 @@ bool sdbus_get_inserted(SDBus *sdbus)
SDState *card = get_card(sdbus);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
return sc->get_inserted(card);
}
@@ -204,7 +205,7 @@ bool sdbus_get_readonly(SDBus *sdbus)
SDState *card = get_card(sdbus);
Re: [PATCH v46 3/5] hw/sd/sdcard: Rename sd_cmd_SEND_OP_COND handler
On 7/3/24 3:43 PM, Philippe Mathieu-Daudé wrote:
The correct command name is 'SD SEND_OP_COND',
rename accordingly.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Cédric Le Goater
Thanks,
C.
---
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 04e8fdb262..10f2764a53 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1701,7 +1701,7 @@ static sd_rsp_type_t
sd_acmd_SET_WR_BLK_ERASE_COUNT(SDState *sd, SDRequest req)
}
/* ACMD41 */
-static sd_rsp_type_t sd_acmd_SD_APP_OP_COND(SDState *sd, SDRequest req)
+static sd_rsp_type_t sd_cmd_SEND_OP_COND(SDState *sd, SDRequest req)
{
if (sd->state != sd_idle_state) {
return sd_invalid_state_for_cmd(sd, req);
@@ -2378,7 +2378,7 @@ static const SDProto sd_proto_sd = {
[13] = {8, sd_adtc, "SD_STATUS", sd_acmd_SD_STATUS},
[22] = {8, sd_adtc, "SEND_NUM_WR_BLOCKS",
sd_acmd_SEND_NUM_WR_BLOCKS},
[23] = {8, sd_ac, "SET_WR_BLK_ERASE_COUNT",
sd_acmd_SET_WR_BLK_ERASE_COUNT},
-[41] = {8, sd_bcr, "SD_APP_OP_COND", sd_acmd_SD_APP_OP_COND},
+[41] = {8, sd_bcr, "SEND_OP_COND", sd_cmd_SEND_OP_COND},
[42] = {8, sd_ac, "SET_CLR_CARD_DETECT",
sd_acmd_SET_CLR_CARD_DETECT},
[51] = {8, sd_adtc, "SEND_SCR", sd_acmd_SEND_SCR},
},
Re: [PATCH v46 4/5] hw/sd/sdcard: Introduce set_csd/set_cid handlers
On 7/3/24 3:43 PM, Philippe Mathieu-Daudé wrote:
In preparation of introducing eMMC support which have
different CSD/CID structures, introduce a pair of handlers
in SDCardClass.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Cédric Le Goater
Thanks,
C.
---
include/hw/sd/sd.h | 2 ++
hw/sd/sd.c | 7 +--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 29c76935a0..bfbc83c110 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -128,6 +128,8 @@ struct SDCardClass {
void (*enable)(SDState *sd, bool enable);
bool (*get_inserted)(SDState *sd);
bool (*get_readonly)(SDState *sd);
+void (*set_cid)(SDState *sd);
+void (*set_csd)(SDState *sd, uint64_t size);
const struct SDProto *proto;
};
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 10f2764a53..d46be50760 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -689,6 +689,7 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
static void sd_reset(DeviceState *dev)
{
SDState *sd = SD_CARD(dev);
+SDCardClass *sc = SD_CARD_GET_CLASS(sd);
uint64_t size;
uint64_t sect;
@@ -709,8 +710,8 @@ static void sd_reset(DeviceState *dev)
sd->size = size;
sd_set_ocr(sd);
sd_set_scr(sd);
-sd_set_cid(sd);
-sd_set_csd(sd, size);
+sc->set_cid(sd);
+sc->set_csd(sd, size);
sd_set_cardstatus(sd);
sd_set_sdstatus(sd);
@@ -2485,6 +2486,8 @@ static void sd_class_init(ObjectClass *klass, void *data)
sc->enable = sd_enable;
sc->get_inserted = sd_get_inserted;
sc->get_readonly = sd_get_readonly;
+sc->set_cid = sd_set_cid;
+sc->set_csd = sd_set_csd;
sc->proto = &sd_proto_sd;
}
[PATCH v46 0/5] hw/sd/sdcard: Cleanups before adding eMMC support
(patches from v42 already reviewed not reposted) Since v45: - RAZ/WI on GEN_CMD (Luc & Manos) - Rename sd_cmd_SEND_OP_COND - Introduce TYPE_SDMMC_COMMON Philippe Mathieu-Daudé (5): hw/sd/sdcard: Use spec v3.01 by default hw/sd/sdcard: Add sd_cmd_GEN_CMD handler (CMD56) hw/sd/sdcard: Rename sd_cmd_SEND_OP_COND handler hw/sd/sdcard: Introduce set_csd/set_cid handlers hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD hw/sd/sdmmc-internal.h | 3 ++ include/hw/sd/sd.h | 2 + hw/core/machine.c | 1 + hw/sd/core.c | 29 ++- hw/sd/sd.c | 113 ++--- 5 files changed, 81 insertions(+), 67 deletions(-) -- 2.41.0
[PATCH v46 5/5] hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD
In order to keep eMMC model simpler to maintain,
extract common properties and the common code from
class_init to the (internal) TYPE_SDMMC_COMMON.
Update the corresponding QOM cast macros.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sdmmc-internal.h | 3 +++
hw/sd/core.c | 29
hw/sd/sd.c | 50 --
3 files changed, 52 insertions(+), 30 deletions(-)
diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
index cc0b69e834..91eb5b6b2f 100644
--- a/hw/sd/sdmmc-internal.h
+++ b/hw/sd/sdmmc-internal.h
@@ -11,6 +11,9 @@
#ifndef SDMMC_INTERNAL_H
#define SDMMC_INTERNAL_H
+#define TYPE_SDMMC_COMMON "sdmmc-common"
+DECLARE_OBJ_CHECKERS(SDState, SDCardClass, SDMMC_COMMON, TYPE_SDMMC_COMMON)
+
/*
* EXT_CSD Modes segment
*
diff --git a/hw/sd/core.c b/hw/sd/core.c
index 52d5d90045..4b30218b52 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -24,6 +24,7 @@
#include "hw/sd/sd.h"
#include "qemu/module.h"
#include "qapi/error.h"
+#include "sdmmc-internal.h"
#include "trace.h"
static inline const char *sdbus_name(SDBus *sdbus)
@@ -39,7 +40,7 @@ static SDState *get_card(SDBus *sdbus)
if (!kid) {
return NULL;
}
-return SD_CARD(kid->child);
+return SDMMC_COMMON(kid->child);
}
uint8_t sdbus_get_dat_lines(SDBus *sdbus)
@@ -48,7 +49,7 @@ uint8_t sdbus_get_dat_lines(SDBus *sdbus)
uint8_t dat_lines = 0b; /* 4 bit bus width */
if (slave) {
-SDCardClass *sc = SD_CARD_GET_CLASS(slave);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(slave);
if (sc->get_dat_lines) {
dat_lines = sc->get_dat_lines(slave);
@@ -65,7 +66,7 @@ bool sdbus_get_cmd_line(SDBus *sdbus)
bool cmd_line = true;
if (slave) {
-SDCardClass *sc = SD_CARD_GET_CLASS(slave);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(slave);
if (sc->get_cmd_line) {
cmd_line = sc->get_cmd_line(slave);
@@ -82,7 +83,7 @@ void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts)
trace_sdbus_set_voltage(sdbus_name(sdbus), millivolts);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
assert(sc->set_voltage);
sc->set_voltage(card, millivolts);
@@ -95,7 +96,7 @@ int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t
*response)
trace_sdbus_command(sdbus_name(sdbus), req->cmd, req->arg);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
return sc->do_command(card, req, response);
}
@@ -109,7 +110,7 @@ void sdbus_write_byte(SDBus *sdbus, uint8_t value)
trace_sdbus_write(sdbus_name(sdbus), value);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
sc->write_byte(card, value);
}
@@ -121,7 +122,7 @@ void sdbus_write_data(SDBus *sdbus, const void *buf, size_t
length)
const uint8_t *data = buf;
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
for (size_t i = 0; i < length; i++) {
trace_sdbus_write(sdbus_name(sdbus), data[i]);
@@ -136,7 +137,7 @@ uint8_t sdbus_read_byte(SDBus *sdbus)
uint8_t value = 0;
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
value = sc->read_byte(card);
}
@@ -151,7 +152,7 @@ void sdbus_read_data(SDBus *sdbus, void *buf, size_t length)
uint8_t *data = buf;
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
for (size_t i = 0; i < length; i++) {
data[i] = sc->read_byte(card);
@@ -165,7 +166,7 @@ bool sdbus_receive_ready(SDBus *sdbus)
SDState *card = get_card(sdbus);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
return sc->receive_ready(card);
}
@@ -178,7 +179,7 @@ bool sdbus_data_ready(SDBus *sdbus)
SDState *card = get_card(sdbus);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
return sc->data_ready(card);
}
@@ -191,7 +192,7 @@ bool sdbus_get_inserted(SDBus *sdbus)
SDState *card = get_card(sdbus);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
return sc->get_inserted(card);
}
@@ -204,7 +205,7 @@ bool sdbus_get_readonly(SDBus *sdbus)
SDState *card = get_card(sdbus);
if (card) {
-SDCardClass *sc = SD_CARD_GET_CLASS(card);
+SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
return sc->get_readonly(car
[PATCH v46 4/5] hw/sd/sdcard: Introduce set_csd/set_cid handlers
In preparation of introducing eMMC support which have
different CSD/CID structures, introduce a pair of handlers
in SDCardClass.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/sd/sd.h | 2 ++
hw/sd/sd.c | 7 +--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 29c76935a0..bfbc83c110 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -128,6 +128,8 @@ struct SDCardClass {
void (*enable)(SDState *sd, bool enable);
bool (*get_inserted)(SDState *sd);
bool (*get_readonly)(SDState *sd);
+void (*set_cid)(SDState *sd);
+void (*set_csd)(SDState *sd, uint64_t size);
const struct SDProto *proto;
};
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 10f2764a53..d46be50760 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -689,6 +689,7 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
static void sd_reset(DeviceState *dev)
{
SDState *sd = SD_CARD(dev);
+SDCardClass *sc = SD_CARD_GET_CLASS(sd);
uint64_t size;
uint64_t sect;
@@ -709,8 +710,8 @@ static void sd_reset(DeviceState *dev)
sd->size = size;
sd_set_ocr(sd);
sd_set_scr(sd);
-sd_set_cid(sd);
-sd_set_csd(sd, size);
+sc->set_cid(sd);
+sc->set_csd(sd, size);
sd_set_cardstatus(sd);
sd_set_sdstatus(sd);
@@ -2485,6 +2486,8 @@ static void sd_class_init(ObjectClass *klass, void *data)
sc->enable = sd_enable;
sc->get_inserted = sd_get_inserted;
sc->get_readonly = sd_get_readonly;
+sc->set_cid = sd_set_cid;
+sc->set_csd = sd_set_csd;
sc->proto = &sd_proto_sd;
}
--
2.41.0
[PATCH v46 3/5] hw/sd/sdcard: Rename sd_cmd_SEND_OP_COND handler
The correct command name is 'SD SEND_OP_COND',
rename accordingly.
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 04e8fdb262..10f2764a53 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1701,7 +1701,7 @@ static sd_rsp_type_t
sd_acmd_SET_WR_BLK_ERASE_COUNT(SDState *sd, SDRequest req)
}
/* ACMD41 */
-static sd_rsp_type_t sd_acmd_SD_APP_OP_COND(SDState *sd, SDRequest req)
+static sd_rsp_type_t sd_cmd_SEND_OP_COND(SDState *sd, SDRequest req)
{
if (sd->state != sd_idle_state) {
return sd_invalid_state_for_cmd(sd, req);
@@ -2378,7 +2378,7 @@ static const SDProto sd_proto_sd = {
[13] = {8, sd_adtc, "SD_STATUS", sd_acmd_SD_STATUS},
[22] = {8, sd_adtc, "SEND_NUM_WR_BLOCKS", sd_acmd_SEND_NUM_WR_BLOCKS},
[23] = {8, sd_ac, "SET_WR_BLK_ERASE_COUNT",
sd_acmd_SET_WR_BLK_ERASE_COUNT},
-[41] = {8, sd_bcr, "SD_APP_OP_COND", sd_acmd_SD_APP_OP_COND},
+[41] = {8, sd_bcr, "SEND_OP_COND", sd_cmd_SEND_OP_COND},
[42] = {8, sd_ac, "SET_CLR_CARD_DETECT",
sd_acmd_SET_CLR_CARD_DETECT},
[51] = {8, sd_adtc, "SEND_SCR", sd_acmd_SEND_SCR},
},
--
2.41.0
[PATCH v46 2/5] hw/sd/sdcard: Add sd_cmd_GEN_CMD handler (CMD56)
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Handle as RAZ/WI.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 54 --
1 file changed, 20 insertions(+), 34 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a08a452d81..04e8fdb262 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -243,7 +243,6 @@ static const char *sd_cmd_name(SDState *sd, uint8_t cmd)
[25]= "WRITE_MULTIPLE_BLOCK",
[26]= "MANUF_RSVD",
[40]= "DPS_spec",
-[56]= "GEN_CMD",
[60]= "MANUF_RSVD", [61]= "MANUF_RSVD",
[62]= "MANUF_RSVD", [63]= "MANUF_RSVD",
};
@@ -902,9 +901,6 @@ static void sd_blk_write(SDState *sd, uint64_t addr,
uint32_t len)
}
}
-#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len)
-#define APP_WRITE_BLOCK(a, len)
-
static void sd_erase(SDState *sd)
{
uint64_t erase_start = sd->erase_start;
@@ -1641,6 +1637,22 @@ static sd_rsp_type_t sd_cmd_APP_CMD(SDState *sd,
SDRequest req)
return sd_r1;
}
+/* CMD56 */
+static sd_rsp_type_t sd_cmd_GEN_CMD(SDState *sd, SDRequest req)
+{
+if (sd->state != sd_transfer_state) {
+return sd_invalid_state_for_cmd(sd, req);
+}
+
+/* Vendor specific command: our model is RAZ/WI */
+if (req.arg & 1) {
+memset(sd->data, 0, sizeof(sd->data));
+return sd_cmd_to_sendingdata(sd, req, 0, NULL, 0);
+} else {
+return sd_cmd_to_receivingdata(sd, req, 0, 0);
+}
+}
+
/* CMD58 */
static sd_rsp_type_t spi_cmd_READ_OCR(SDState *sd, SDRequest req)
{
@@ -1836,22 +1848,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
SDRequest req)
case 26: /* CMD26: PROGRAM_CID */
return sd_cmd_to_receivingdata(sd, req, 0, sizeof(sd->cid));
-/* Application specific commands (Class 8) */
-case 56: /* CMD56: GEN_CMD */
-switch (sd->state) {
-case sd_transfer_state:
-sd->data_offset = 0;
-if (req.arg & 1)
-sd->state = sd_sendingdata_state;
-else
-sd->state = sd_receivingdata_state;
-return sd_r1;
-
-default:
-break;
-}
-break;
-
default:
qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
return sd_illegal;
@@ -2187,11 +2183,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 56: /* CMD56: GEN_CMD */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
-APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
-sd->state = sd_transfer_state;
-}
+sd_generic_write_byte(sd, value);
break;
default:
@@ -2233,6 +2225,7 @@ uint8_t sd_read_byte(SDState *sd)
case 22: /* ACMD22: SEND_NUM_WR_BLOCKS */
case 30: /* CMD30: SEND_WRITE_PROT */
case 51: /* ACMD51: SEND_SCR */
+case 56: /* CMD56: GEN_CMD */
sd_generic_read_byte(sd, &ret);
break;
@@ -2260,15 +2253,6 @@ uint8_t sd_read_byte(SDState *sd)
}
break;
-case 56: /* CMD56: GEN_CMD */
-if (sd->data_offset == 0)
-APP_READ_BLOCK(sd->data_start, sd->blk_len);
-ret = sd->data[sd->data_offset ++];
-
-if (sd->data_offset >= sd->blk_len)
-sd->state = sd_transfer_state;
-break;
-
default:
qemu_log_mask(LOG_GUEST_ERROR, "%s: unknown command\n", __func__);
return 0x00;
@@ -2323,6 +2307,7 @@ static const SDProto sd_proto_spi = {
[52] = {9, sd_spi, "IO_RW_DIRECT", sd_cmd_optional},
[53] = {9, sd_spi, "IO_RW_EXTENDED", sd_cmd_optional},
[55] = {8, sd_spi, "APP_CMD", sd_cmd_APP_CMD},
+[56] = {8, sd_spi, "GEN_CMD", sd_cmd_GEN_CMD},
[57] = {10, sd_spi, "DIRECT_SECURE_WRITE", sd_cmd_optional},
[58] = {0, sd_spi, "READ_OCR", spi_cmd_READ_OCR},
[59] = {0, sd_spi, "CRC_ON_OFF", spi_cmd_CRC_ON_OFF},
@@ -2383,6 +2368,7 @@ static const SDProto sd_proto_sd = {
[52] = {9, sd_bc, "IO_RW_DIRECT", sd_cmd_optional},
[53] = {9, sd_bc, "IO_RW_EXTENDED", sd_cmd_optional},
[55] = {8, sd_ac, "APP_CMD", sd_cmd_APP_CMD},
+[56] = {8, sd_adtc, "GEN_CMD", sd_cmd_GEN_CMD},
[57] = {10, sd_adtc, "DIRECT_SECURE_WRITE", sd_cmd_optional},
[58] = {11, sd_adtc, "READ_EXTR_MULTI", sd_cmd_optional},
[59] = {11, sd_adtc, "WRITE_EXTR_MULTI", sd_cmd_optional},
--
2.41.0
[PATCH v46 1/5] hw/sd/sdcard: Use spec v3.01 by default
Recent SDHCI expect cards to support the v3.01 spec
to negociate lower I/O voltage. Select it by default.
Versioned machine types with a version of 9.0 or
earlier retain the old default (spec v2.00).
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Cédric Le Goater
---
v43: update versioned machines (danpb)
Cc: Daniel P . Berrangé
---
hw/core/machine.c | 1 +
hw/sd/sd.c| 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 655d75c21f..4377f943d5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -38,6 +38,7 @@ GlobalProperty hw_compat_9_0[] = {
{"arm-cpu", "backcompat-cntfrq", "true" },
{"scsi-disk-base", "migrate-emulated-scsi-request", "false" },
{"vfio-pci", "skip-vsc-check", "false" },
+{"sd-card", "spec_version", "2" },
};
const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 53767beaf8..a08a452d81 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2471,7 +2471,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
static Property sd_properties[] = {
DEFINE_PROP_UINT8("spec_version", SDState,
- spec_version, SD_PHY_SPECv2_00_VERS),
+ spec_version, SD_PHY_SPECv3_01_VERS),
DEFINE_PROP_DRIVE("drive", SDState, blk),
DEFINE_PROP_END_OF_LIST()
};
--
2.41.0
Re: [PATCH v10 11/12] hw/pci: Convert rom_bar into OnOffAuto
On Wed, Jul 03, 2024 at 01:00:21PM +0200, BALATON Zoltan wrote:
> On Wed, 3 Jul 2024, Michael S. Tsirkin wrote:
> > On Wed, Jul 03, 2024 at 04:15:23AM +0200, BALATON Zoltan wrote:
> > > On Tue, 2 Jul 2024, Michael S. Tsirkin wrote:
> > > > On Thu, Jun 27, 2024 at 03:08:00PM +0900, Akihiko Odaki wrote:
> > > > > rom_bar is tristate but was defined as uint32_t so convert it into
> > > > > OnOffAuto.
> > > > >
> > > > > Signed-off-by: Akihiko Odaki
> > > >
> > > > Commit log should explain why this is an improvement,
> > > > not just what's done.
> > > >
> > > >
> > > > > diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
> > > > > index e17bb50789ad..35c6c8e28493 100644
> > > > > --- a/docs/igd-assign.txt
> > > > > +++ b/docs/igd-assign.txt
> > > > > @@ -35,7 +35,7 @@ IGD has two different modes for assignment using
> > > > > vfio-pci:
> > > > >ISA/LPC bridge device (vfio-pci-igd-lpc-bridge) on the root
> > > > > bus at
> > > > >PCI address 1f.0.
> > > > > * The IGD device must have a VGA ROM, either provided via the
> > > > > romfile
> > > > > - option or loaded automatically through vfio (standard).
> > > > > rombar=0
> > > > > + option or loaded automatically through vfio (standard).
> > > > > rombar=off
> > > > >will disable legacy mode support.
> > > > > * Hotplug of the IGD device is not supported.
> > > > > * The IGD device must be a SandyBridge or newer model device.
> > > >
> > > > ...
> > > >
> > > > > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> > > > > index 39dae72497e0..0e920ed0691a 100644
> > > > > --- a/hw/vfio/pci-quirks.c
> > > > > +++ b/hw/vfio/pci-quirks.c
> > > > > @@ -33,7 +33,7 @@
> > > > > * execution as noticed with the BCM 57810 card for lack of a
> > > > > * more better way to handle such issues.
> > > > > * The user can still override by specifying a romfile or
> > > > > - * rombar=1.
> > > > > + * rombar=on.
> > > > > * Please see https://bugs.launchpad.net/qemu/+bug/1284874
> > > > > * for an analysis of the 57810 card hang. When adding
> > > > > * a new vendor id/device id combination below, please also add
> > > >
> > > >
> > > > So we are apparently breaking a bunch of users who followed
> > > > documentation to the dot. Why is this a good idea?
> > >
> > > On/off is clearer than 1/0. But isn't 1/0 a synonym for on/off so previous
> > > command lines would still work?
> > >
> > > Regards,
> > > BALATON Zoltan
> >
> > I see nothing in code that would make it so:
> >
> >
> > const QEnumLookup OnOffAuto_lookup = {
> >.array = (const char *const[]) {
> >[ON_OFF_AUTO_AUTO] = "auto",
> >[ON_OFF_AUTO_ON] = "on",
> >[ON_OFF_AUTO_OFF] = "off",
> >},
> >.size = ON_OFF_AUTO__MAX
> > };
> >
> > I also tried with an existing property:
> >
> > $ ./qemu-system-x86_64 -device intel-hda,msi=0
> > qemu-system-x86_64: -device intel-hda,msi=0: Parameter 'msi' does not
> > accept value '0'
>
> Then it was probably bit properties that also accept 0/1, on/off,
> true/false.
I mean, the code is open, why do you keep guessing?
No, these reuse the bool parsing logic:
static void prop_get_bit(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
Property *prop = opaque;
uint32_t *p = object_field_prop_ptr(obj, prop);
bool value = (*p & qdev_get_prop_mask(prop)) != 0;
visit_type_bool(v, name, &value, errp);
}
and that never accepted 0 or 1:
bool qapi_bool_parse(const char *name, const char *value, bool *obj, Error
**errp)
{
if (g_str_equal(value, "on") ||
g_str_equal(value, "yes") ||
g_str_equal(value, "true") ||
g_str_equal(value, "y")) {
*obj = true;
return true;
}
if (g_str_equal(value, "off") ||
g_str_equal(value, "no") ||
g_str_equal(value, "false") ||
g_str_equal(value, "n")) {
*obj = false;
return true;
}
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
"'on' or 'off'");
return false;
}
> Maybe similar aliases could be added to on/off/auto?
Could be, but even then switching to that would mean that user sets 1
but query returns "on". Might or might not surprise some users.
Adding true/false yes/no y/n aliases to on/off/auto might make sense
though, for consistency. Donnu if QAPI guys will agree, though,
and not directly related to this patchset.
One other idea is to add a generic way to detect that a property is set
by user. This requirement comes up, once in a while.
> In any case when I first saw rombar I thought it would set the BAR of the
> ROM so wondered why it's 1 and not 5 or 6 or an offset. So on/off is clearer
> in this case.
>
> Regards,
> BALATON Zoltan
I agree here, but it's been here for a long time.
--
MST
Re: [PATCH v45 3/3] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
On 3/7/24 14:24, Manos Pitsidianakis wrote: On Wed, 03 Jul 2024 11:59, Philippe Mathieu-Daudé wrote: "General command" (GEN_CMD, CMD56) is described as: GEN_CMD is the same as the single block read or write commands (CMD24 or CMD17). The difference is that [...] the data block is not a memory payload data but has a vendor specific format and meaning. Thus this block must not be stored overwriting data block on underlying storage drive. Keep it in a dedicated 'vendor_data[]' array. I am reading the 4.3 spec, and it says: The bus transaction of the GEN_CMD is the same as the single block read or write commands (CMD24 or CMD17). The difference is that the argument denotes the direction of the data transfer (rather than the address) and the data block is not a memory payload data but has a vendor specific format and meaning. The vendor here (qemu) does not support any functionality with CMD56. I think the correct approach would be to read zeros and discard writes, without storing anything and without changing data_offset (which is for `data` buffer) What do you think? As Luc suggested in v42. Indeed simpler thus clever. Signed-off-by: Philippe Mathieu-Daudé --- v43: Do not re-use VMSTATE_UNUSED_V (danpb) v44: Use subsection (Luc) v45: Remove APP_READ_BLOCK/APP_WRITE_BLOCK macros --- hw/sd/sd.c | 29 +++-- 1 file changed, 19 insertions(+), 10 deletions(-)
Re: [PATCH v2 2/5] hw/net:ftgmac100: support 64 bits dma dram address for AST2700
On 7/3/24 10:16 AM, Jamin Lin wrote:
ASPEED AST2700 SOC is a 64 bits quad core CPUs (Cortex-a35)
And the base address of dram is "0x4 " which
is 64bits address.
It have "Normal Priority Transmit Ring Base Address Register High(0x17C)",
"High Priority Transmit Ring Base Address Register High(0x184)" and
"Receive Ring Base Address Register High(0x18C)" to save the high part physical
address of descriptor manager.
Ex: TX descriptor manager address [34:0]
The "Normal Priority Transmit Ring Base Address Register High(0x17C)"
bits [2:0] which corresponds the bits [34:32] of the 64 bits address of
the TX ring buffer address.
The "Normal Priority Transmit Ring Base Address Register(0x20)" bits [31:0]
which corresponds the bits [31:0] of the 64 bits address
of the TX ring buffer address.
Besides, it have "TXDES 2" and "RXDES 2"
to save the high part physical address of packet buffer.
Ex: TX packet buffer address [34:0]
The "TXDES 2" bits [18:16] which corresponds the bits [34:32]
of the 64 bits address of the TX packet buffer address
and "TXDES 3" bits [31:0] which corresponds the bits [31:0]
of the 64 bits address of the TX packet buffer address.
Update TX/RX ring and descriptor data type to uint64_t
and supports TX/RX ring, descriptor and packet buffers
64 bits address for all ASPEED SOCs models.
Incrementing the version of vmstate to 2.
Introduce a new class(ftgmac100_high),
class attribute and memop handlers,
the realize handler instantiate a new memory
region and map it on top of the current one to
read/write FTGMAC100_*_HIGH regs.
Signed-off-by: Jamin Lin
---
hw/net/ftgmac100.c | 156 -
include/hw/net/ftgmac100.h | 24 --
2 files changed, 151 insertions(+), 29 deletions(-)
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 4e88430b2f..3d13f54efc 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -56,6 +56,16 @@
#define FTGMAC100_PHYDATA 0x64
#define FTGMAC100_FCR 0x68
+/*
+ * FTGMAC100 High registers
+ *
+ * values below are offset by - FTGMAC100_HIGH_OFFSET from datasheet
+ * because its memory region is start at FTGMAC100_HIGH_OFFSET
+ */
+#define FTGMAC100_NPTXR_BADR_HIGH (0x17C - FTGMAC100_HIGH_OFFSET)
+#define FTGMAC100_HPTXR_BADR_HIGH (0x184 - FTGMAC100_HIGH_OFFSET)
+#define FTGMAC100_RXR_BADR_HIGH (0x18C - FTGMAC100_HIGH_OFFSET)
+
/*
* Interrupt status register & interrupt enable register
*/
@@ -165,6 +175,8 @@
#define FTGMAC100_TXDES1_TX2FIC (1 << 30)
#define FTGMAC100_TXDES1_TXIC(1 << 31)
+#define FTGMAC100_TXDES2_TXBUF_BADR_HI(x) (((x) >> 16) & 0x7)
+
/*
* Receive descriptor
*/
@@ -198,13 +210,15 @@
#define FTGMAC100_RXDES1_UDP_CHKSUM_ERR (1 << 26)
#define FTGMAC100_RXDES1_IP_CHKSUM_ERR (1 << 27)
+#define FTGMAC100_RXDES2_RXBUF_BADR_HI(x) (((x) >> 16) & 0x7)
+
/*
* Receive and transmit Buffer Descriptor
*/
typedef struct {
uint32_tdes0;
uint32_tdes1;
-uint32_tdes2;/* not used by HW */
+uint32_tdes2;/* used by HW high address */
uint32_tdes3;
} FTGMAC100Desc;
@@ -515,12 +529,14 @@ out:
return frame_size;
}
-static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
-uint32_t tx_descriptor)
+static void ftgmac100_do_tx(FTGMAC100State *s, uint64_t tx_ring,
+uint64_t tx_descriptor)
{
+FTGMAC100Class *fc = FTGMAC100_GET_CLASS(s);
int frame_size = 0;
uint8_t *ptr = s->frame;
-uint32_t addr = tx_descriptor;
+uint64_t addr = tx_descriptor;
+uint64_t buf_addr = 0;
To ease reading, I would extract in a standalone patch the part converting
addresses to 64 bits.
uint32_t flags = 0;
while (1) {
@@ -559,7 +575,12 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t
tx_ring,
len = sizeof(s->frame) - frame_size;
}
-if (dma_memory_read(&address_space_memory, bd.des3,
+buf_addr = bd.des3;
+if (fc->is_dma64) {
+buf_addr = deposit64(buf_addr, 32, 32,
+ FTGMAC100_TXDES2_TXBUF_BADR_HI(bd.des2));
+}
+if (dma_memory_read(&address_space_memory, buf_addr,
ptr, len, MEMTXATTRS_UNSPECIFIED)) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to read packet @
0x%x\n",
__func__, bd.des3);
@@ -726,9 +747,9 @@ static uint64_t ftgmac100_read(void *opaque, hwaddr addr,
unsigned size)
case FTGMAC100_MATH1:
return s->math[1];
case FTGMAC100_RXR_BADR:
-return s->rx_ring;
+return extract64(s->rx_ring, 0, 32);
case FTGMAC100_NPTXR_BADR:
-return s->tx_ring;
+return extract64(s->tx_ring, 0, 32);
case FTGMAC100_ITC:
return s->itc;
case FTGMAC100_DBLAC:
@@ -799,9 +820,8 @@ stati
Re: [PATCH v45 3/3] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
On Wed, 03 Jul 2024 11:59, Philippe Mathieu-Daudé wrote:
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Keep it in a dedicated
'vendor_data[]' array.
I am reading the 4.3 spec, and it says:
The bus transaction of the GEN_CMD is the same as the single block
read or write commands (CMD24 or CMD17). The difference is that the
argument denotes the direction of the data transfer (rather than the
address) and the data block is not a memory payload data but has a
vendor specific format and meaning.
The vendor here (qemu) does not support any functionality with CMD56. I
think the correct approach would be to read zeros and discard writes,
without storing anything and without changing data_offset (which is for
`data` buffer)
What do you think?
Signed-off-by: Philippe Mathieu-Daudé
---
v43: Do not re-use VMSTATE_UNUSED_V (danpb)
v44: Use subsection (Luc)
v45: Remove APP_READ_BLOCK/APP_WRITE_BLOCK macros
---
hw/sd/sd.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a08a452d81..5d58937dd4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -153,6 +153,8 @@ struct SDState {
uint32_t data_offset;
size_t data_size;
uint8_t data[512];
+uint8_t vendor_data[512];
+
qemu_irq readonly_cb;
qemu_irq inserted_cb;
QEMUTimer *ocr_power_timer;
@@ -719,6 +721,7 @@ static void sd_reset(DeviceState *dev)
sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
sd->wp_group_bits = sect;
sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
+memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
memset(sd->function_group, 0, sizeof(sd->function_group));
sd->erase_start = INVALID_ADDRESS;
sd->erase_end = INVALID_ADDRESS;
@@ -793,6 +796,16 @@ static const VMStateDescription sd_ocr_vmstate = {
},
};
+static const VMStateDescription sd_vendordata_vmstate = {
+.name = "sd-card/vendor_data-state",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (const VMStateField[]) {
+VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
+VMSTATE_END_OF_LIST()
+},
+};
+
static int sd_vmstate_pre_load(void *opaque)
{
SDState *sd = opaque;
@@ -840,6 +853,7 @@ static const VMStateDescription sd_vmstate = {
},
.subsections = (const VMStateDescription * const []) {
&sd_ocr_vmstate,
+&sd_vendordata_vmstate,
NULL
},
};
@@ -902,9 +916,6 @@ static void sd_blk_write(SDState *sd, uint64_t addr,
uint32_t len)
}
}
-#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len)
-#define APP_WRITE_BLOCK(a, len)
-
static void sd_erase(SDState *sd)
{
uint64_t erase_start = sd->erase_start;
@@ -2187,9 +2198,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 56: /* CMD56: GEN_CMD */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
-APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
+sd->vendor_data[sd->data_offset++] = value;
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
}
break;
@@ -2261,12 +2271,11 @@ uint8_t sd_read_byte(SDState *sd)
break;
case 56: /* CMD56: GEN_CMD */
-if (sd->data_offset == 0)
-APP_READ_BLOCK(sd->data_start, sd->blk_len);
-ret = sd->data[sd->data_offset ++];
+ret = sd->vendor_data[sd->data_offset++];
-if (sd->data_offset >= sd->blk_len)
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
+}
break;
default:
--
2.41.0
[PATCH v2 5/5] test/avocado/machine_aspeed.py: update to test network for AST2700
Update a test case to test network connection via ssh and
changes to test Aspeed OpenBMC SDK v09.02 for AST2700.
ASPEED fixed TX mask issue from linux/drivers/ftgmac100.c.
It is required to use ASPEED SDK image since v09.02
for AST2700 QEMU network testing.
A test image is downloaded from the ASPEED Forked OpenBMC GitHub
release repository :
https://github.com/AspeedTech-BMC/openbmc/releases/
Test command:
```
cd build
pyvenv/bin/avocado run
../qemu/tests/avocado/machine_aspeed.py:AST2x00MachineSDK.test_aarch64_ast2700_evb_sdk_v09_02
```
Signed-off-by: Jamin Lin
---
tests/avocado/machine_aspeed.py | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/tests/avocado/machine_aspeed.py b/tests/avocado/machine_aspeed.py
index 3a20644fb2..855da805ae 100644
--- a/tests/avocado/machine_aspeed.py
+++ b/tests/avocado/machine_aspeed.py
@@ -313,14 +313,14 @@ def do_test_arm_aspeed_sdk_start(self, image):
def do_test_aarch64_aspeed_sdk_start(self, image):
self.vm.set_console()
-self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw')
+self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
+ '-net', 'nic', '-net',
'user,hostfwd=:127.0.0.1:0-:22')
self.vm.launch()
self.wait_for_console_pattern('U-Boot 2023.10')
self.wait_for_console_pattern('## Loading kernel from FIT Image')
self.wait_for_console_pattern('Starting kernel ...')
-self.wait_for_console_pattern("systemd[1]: Hostname set to")
@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on
GitLab')
@@ -387,15 +387,15 @@ def test_arm_ast2600_evb_sdk(self):
year = time.strftime("%Y")
self.ssh_command_output_contains('/sbin/hwclock -f /dev/rtc1', year);
-def test_aarch64_ast2700_evb_sdk_v09_01(self):
+def test_aarch64_ast2700_evb_sdk_v09_02(self):
"""
:avocado: tags=arch:aarch64
:avocado: tags=machine:ast2700-evb
"""
image_url = ('https://github.com/AspeedTech-BMC/openbmc/releases/'
- 'download/v09.01/ast2700-default-obmc.tar.gz')
-image_hash =
'b1cc0fd73c7650d34c9c8459a243f52a91e9e27144b8608b2645ab19461d1e07'
+ 'download/v09.02/ast2700-default-obmc.tar.gz')
+image_hash =
'ac969c2602f4e6bdb69562ff466b89ae3fe1d86e1f6797bb7969d787f82116a7'
image_path = self.fetch_asset(image_url, asset_hash=image_hash,
algorithm='sha256')
archive.extract(image_path, self.workdir)
@@ -436,4 +436,5 @@ def test_aarch64_ast2700_evb_sdk_v09_01(self):
self.vm.add_args('-smp', str(num_cpu))
self.do_test_aarch64_aspeed_sdk_start(image_dir + 'image-bmc')
-
+self.wait_for_console_pattern('nodistro.0 ast2700-default ttyS12')
+self.ssh_connect('root', '0penBmc', False)
--
2.34.1
[PATCH v2 0/5] support AST2700 network
change from v1: - ftgmac100 - fix coding style - support 64 bits dma dram address for AST2700 change from v2: - ftgmac100: update memory region size to 0x200. - ftgmac100: introduce a new class(ftgmac100_high), class attribute and memop handlers, for FTGMAC100_*_HIGH regs read/write. - aspeed_ast27x0: update network model to ftgmac100_high to support 64 bits dram address DMA. - m25p80: support quad mode for w25q01jvq Jamin Lin (5): hw/net:ftgmac100: update memory region size to 0x200 hw/net:ftgmac100: support 64 bits dma dram address for AST2700 aspeed/soc: update to ftgmac100_high model for AST2700 hw/block: m25p80: support quad mode for w25q01jvq test/avocado/machine_aspeed.py: update to test network for AST2700 hw/arm/aspeed_ast27x0.c | 3 +- hw/block/m25p80.c | 16 hw/net/ftgmac100.c | 158 +++- include/hw/net/ftgmac100.h | 24 +++-- tests/avocado/machine_aspeed.py | 13 +-- 5 files changed, 178 insertions(+), 36 deletions(-) -- 2.34.1
RE: [PATCH v2 1/5] hw/net:ftgmac100: update memory region size to 0x200
Hi Cedric,
> Subject: Re: [PATCH v2 1/5] hw/net:ftgmac100: update memory region size to
> 0x200
>
> On 7/3/24 10:16 AM, Jamin Lin wrote:
> > According to the datasheet of ASPEED SOCs, one MAC controller owns
> > 128KB of register space for AST2500.
> >
> > However, one MAC controller only owns 64KB of register space for
> > AST2600 and AST2700.
> >
> > It set the memory region size 128KB and it occupied another
> > controllers Address Spaces.
> >
> > Currently, the ftgmac100 model use 0x100 register space.
> > To support DMA 64 bits dram address and new future
> > mode(ftgmac100_high) which have "Normal Priority Transmit Ring Base
> > Address Register High(0x17C)", "High Priority Transmit Ring Base
> > Address Register High(0x184)" and "Receive Ring Base Address Register
> > High(0x18C)" to save the high part physical address of descriptor manager.
> >
> > Update memory region size to 0x200.
> >
> > Signed-off-by: Jamin Lin
> > ---
> > hw/net/ftgmac100.c | 2 +-
> > include/hw/net/ftgmac100.h | 2 ++
> > 2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c index
> > 25e4c0cd5b..4e88430b2f 100644
> > --- a/hw/net/ftgmac100.c
> > +++ b/hw/net/ftgmac100.c
> > @@ -1108,7 +1108,7 @@ static void ftgmac100_realize(DeviceState *dev,
> Error **errp)
> > }
> >
> > memory_region_init_io(&s->iomem, OBJECT(dev), &ftgmac100_ops,
> s,
> > - TYPE_FTGMAC100, 0x2000);
> > + TYPE_FTGMAC100,
> FTGMAC100_NR_REGS);
> > sysbus_init_mmio(sbd, &s->iomem);
> > sysbus_init_irq(sbd, &s->irq);
> > qemu_macaddr_default_if_unset(&s->conf.macaddr);
> > diff --git a/include/hw/net/ftgmac100.h b/include/hw/net/ftgmac100.h
> > index 765d1538a4..5a970676da 100644
> > --- a/include/hw/net/ftgmac100.h
> > +++ b/include/hw/net/ftgmac100.h
> > @@ -14,6 +14,8 @@
> > #define TYPE_FTGMAC100 "ftgmac100"
> > OBJECT_DECLARE_SIMPLE_TYPE(FTGMAC100State, FTGMAC100)
> >
> > +#define FTGMAC100_NR_REGS 0x200
>
> Since this value will size a memory region, I think the define name should be
> changed to FTGMAC100_{MEM,REGION,MMIO}_SIZE. What ever you prefer.
>
Will fix
Thanks-Jamin
>
> Thanks,
>
> C.
>
>
>
> > +
> > #include "hw/sysbus.h"
> > #include "net/net.h"
> >
[PATCH v2 1/5] hw/net:ftgmac100: update memory region size to 0x200
According to the datasheet of ASPEED SOCs, one MAC controller owns 128KB of register space for AST2500. However, one MAC controller only owns 64KB of register space for AST2600 and AST2700. It set the memory region size 128KB and it occupied another controllers Address Spaces. Currently, the ftgmac100 model use 0x100 register space. To support DMA 64 bits dram address and new future mode(ftgmac100_high) which have "Normal Priority Transmit Ring Base Address Register High(0x17C)", "High Priority Transmit Ring Base Address Register High(0x184)" and "Receive Ring Base Address Register High(0x18C)" to save the high part physical address of descriptor manager. Update memory region size to 0x200. Signed-off-by: Jamin Lin --- hw/net/ftgmac100.c | 2 +- include/hw/net/ftgmac100.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c index 25e4c0cd5b..4e88430b2f 100644 --- a/hw/net/ftgmac100.c +++ b/hw/net/ftgmac100.c @@ -1108,7 +1108,7 @@ static void ftgmac100_realize(DeviceState *dev, Error **errp) } memory_region_init_io(&s->iomem, OBJECT(dev), &ftgmac100_ops, s, - TYPE_FTGMAC100, 0x2000); + TYPE_FTGMAC100, FTGMAC100_NR_REGS); sysbus_init_mmio(sbd, &s->iomem); sysbus_init_irq(sbd, &s->irq); qemu_macaddr_default_if_unset(&s->conf.macaddr); diff --git a/include/hw/net/ftgmac100.h b/include/hw/net/ftgmac100.h index 765d1538a4..5a970676da 100644 --- a/include/hw/net/ftgmac100.h +++ b/include/hw/net/ftgmac100.h @@ -14,6 +14,8 @@ #define TYPE_FTGMAC100 "ftgmac100" OBJECT_DECLARE_SIMPLE_TYPE(FTGMAC100State, FTGMAC100) +#define FTGMAC100_NR_REGS 0x200 + #include "hw/sysbus.h" #include "net/net.h" -- 2.34.1
[PATCH v2 4/5] hw/block: m25p80: support quad mode for w25q01jvq
According to the w25q01jv datasheet at page 16,
it is required to set QE bit in "Status Register 2".
Besides, users are able to utilize "Write Status Register 1(0x01)"
command to set QE bit in "Status Register 2" and
utilize "Read Status Register 2(0x35)" command to get the QE bit status.
To support quad mode for w25q01jvq, update collecting data needed
2 bytes for WRSR command in decode_new_cmd function and
verify QE bit at the second byte of collecting data bit 2
in complete_collecting_data.
Update RDCR_EQIO command to set bit 2 of return data
if quad mode enable in decode_new_cmd.
Signed-off-by: Troy Lee
Signed-off-by: Jamin Lin
---
hw/block/m25p80.c | 16
1 file changed, 16 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8dec134832..9e99107b42 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -416,6 +416,7 @@ typedef enum {
/*
* Micron: 0x35 - enable QPI
* Spansion: 0x35 - read control register
+ * Winbond: 0x35 - quad enable
*/
RDCR_EQIO = 0x35,
RSTQIO = 0xf5,
@@ -798,6 +799,11 @@ static void complete_collecting_data(Flash *s)
s->four_bytes_address_mode = extract32(s->data[1], 5, 1);
}
break;
+case MAN_WINBOND:
+if (s->len > 1) {
+s->quad_enable = !!(s->data[1] & 0x02);
+}
+break;
default:
break;
}
@@ -1254,6 +1260,10 @@ static void decode_new_cmd(Flash *s, uint32_t value)
s->needed_bytes = 2;
s->state = STATE_COLLECTING_VAR_LEN_DATA;
break;
+case MAN_WINBOND:
+s->needed_bytes = 2;
+s->state = STATE_COLLECTING_VAR_LEN_DATA;
+break;
default:
s->needed_bytes = 1;
s->state = STATE_COLLECTING_DATA;
@@ -1431,6 +1441,12 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case MAN_MACRONIX:
s->quad_enable = true;
break;
+case MAN_WINBOND:
+s->data[0] = (!!s->quad_enable) << 1;
+s->pos = 0;
+s->len = 1;
+s->state = STATE_READING_DATA;
+break;
default:
break;
}
--
2.34.1
Re: [PATCH v43 2/2] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
On 18:10 Tue 02 Jul , Philippe Mathieu-Daudé wrote:
> "General command" (GEN_CMD, CMD56) is described as:
>
> GEN_CMD is the same as the single block read or write
> commands (CMD24 or CMD17). The difference is that [...]
> the data block is not a memory payload data but has a
> vendor specific format and meaning.
>
> Thus this block must not be stored overwriting data block
> on underlying storage drive. Keep it in a dedicated
> 'vendor_data[]' array.
>
> Signed-off-by: Philippe Mathieu-Daudé
> Tested-by: Cédric Le Goater
> ---
> v43: Do not re-use VMSTATE_UNUSED_V (danpb)
> ---
> 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 808dc1cea6..418ccb14a4 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -153,6 +153,8 @@ struct SDState {
> uint32_t data_offset;
> size_t data_size;
> uint8_t data[512];
> +uint8_t vendor_data[512];
> +
> qemu_irq readonly_cb;
> qemu_irq inserted_cb;
> QEMUTimer *ocr_power_timer;
> @@ -719,6 +721,7 @@ static void sd_reset(DeviceState *dev)
> sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
> sd->wp_group_bits = sect;
> sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
> +memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
> memset(sd->function_group, 0, sizeof(sd->function_group));
> sd->erase_start = INVALID_ADDRESS;
> sd->erase_end = INVALID_ADDRESS;
> @@ -835,6 +838,7 @@ static const VMStateDescription sd_vmstate = {
> VMSTATE_UINT32(data_offset, SDState),
> VMSTATE_UINT8_ARRAY(data, SDState, 512),
> VMSTATE_UNUSED_V(1, 512),
> +VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
Don't you need to bump the VMState version then?
> VMSTATE_BOOL(enable, SDState),
> VMSTATE_END_OF_LIST()
> },
> @@ -2187,9 +2191,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
> break;
>
> case 56: /* CMD56: GEN_CMD */
> -sd->data[sd->data_offset ++] = value;
> -if (sd->data_offset >= sd->blk_len) {
> -APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
> +sd->vendor_data[sd->data_offset ++] = value;
> +if (sd->data_offset >= sizeof(sd->vendor_data)) {
> sd->state = sd_transfer_state;
> }
> break;
> @@ -2261,12 +2264,11 @@ uint8_t sd_read_byte(SDState *sd)
> break;
>
> case 56: /* CMD56: GEN_CMD */
> -if (sd->data_offset == 0)
> -APP_READ_BLOCK(sd->data_start, sd->blk_len);
> -ret = sd->data[sd->data_offset ++];
> +ret = sd->vendor_data[sd->data_offset ++];
>
> -if (sd->data_offset >= sd->blk_len)
> +if (sd->data_offset >= sizeof(sd->vendor_data)) {
> sd->state = sd_transfer_state;
> +}
> break;
>
> default:
> --
> 2.41.0
>
--
[PATCH v2 3/5] aspeed/soc: update to ftgmac100_high model for AST2700
ASPEED AST2700 SOC is a 64 bits quad core CPUs (Cortex-a35)
And the base address of dram is "0x4 " which
is 64bits address.
Update its network model to ftgmac100_high to support
64bits dram address DMA.
Signed-off-by: Jamin Lin
---
hw/arm/aspeed_ast27x0.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 18e6a8b10c..04604a4bef 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -332,7 +332,7 @@ static void aspeed_soc_ast2700_init(Object *obj)
for (i = 0; i < sc->macs_num; i++) {
object_initialize_child(obj, "ftgmac100[*]", &s->ftgmac100[i],
-TYPE_FTGMAC100);
+TYPE_FTGMAC100_HIGH);
object_initialize_child(obj, "mii[*]", &s->mii[i], TYPE_ASPEED_MII);
}
@@ -552,6 +552,7 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev,
Error **errp)
return;
}
+/* Net */
for (i = 0; i < sc->macs_num; i++) {
object_property_set_bool(OBJECT(&s->ftgmac100[i]), "aspeed", true,
&error_abort);
--
2.34.1
RE: [PATCH v2 5/5] test/avocado/machine_aspeed.py: update to test network for AST2700
> -Original Message-
> From: Cédric Le Goater
> Sent: Wednesday, July 3, 2024 5:18 PM
> To: Jamin Lin ; Peter Maydell
> ; Steven Lee ; Troy
> Lee ; Andrew Jeffery ;
> Joel Stanley ; Alistair Francis ;
> Kevin
> Wolf ; Hanna Reitz ; Jason Wang
> ; Cleber Rosa ; Philippe
> Mathieu-Daudé ; Wainer dos Santos Moschetta
> ; Beraldo Leal ; open list:ASPEED
> BMCs ; open list:All patches CC here
> ; open list:Block layer core
>
> Cc: Troy Lee ; Yunlin Tang
>
> Subject: Re: [PATCH v2 5/5] test/avocado/machine_aspeed.py: update to test
> network for AST2700
>
> On 7/3/24 10:16 AM, Jamin Lin wrote:
> > Update a test case to test network connection via ssh and changes to
> > test Aspeed OpenBMC SDK v09.02 for AST2700.
> >
> > ASPEED fixed TX mask issue from linux/drivers/ftgmac100.c.
> > It is required to use ASPEED SDK image since v09.02 for AST2700 QEMU
> > network testing.
> >
> > A test image is downloaded from the ASPEED Forked OpenBMC GitHub
> > release repository :
> > https://github.com/AspeedTech-BMC/openbmc/releases/
> >
> > Test command:
> > ```
> > cd build
> > pyvenv/bin/avocado run
> > ../qemu/tests/avocado/machine_aspeed.py:AST2x00MachineSDK.test_aarch
> 64
> > _ast2700_evb_sdk_v09_02
> > ```
> >
> > Signed-off-by: Jamin Lin
>
> Could you please split the patch ? The change of SDK should be a standalone
> patch.
>
Will fix
Thanks-Jamin
>
> Thanks,
>
> C.
>
>
> > ---
> > tests/avocado/machine_aspeed.py | 13 +++--
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/tests/avocado/machine_aspeed.py
> > b/tests/avocado/machine_aspeed.py index 3a20644fb2..855da805ae 100644
> > --- a/tests/avocado/machine_aspeed.py
> > +++ b/tests/avocado/machine_aspeed.py
> > @@ -313,14 +313,14 @@ def do_test_arm_aspeed_sdk_start(self, image):
> >
> > def do_test_aarch64_aspeed_sdk_start(self, image):
> > self.vm.set_console()
> > -self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw')
> > +self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
> > + '-net', 'nic', '-net',
> > + 'user,hostfwd=:127.0.0.1:0-:22')
> >
> > self.vm.launch()
> >
> > self.wait_for_console_pattern('U-Boot 2023.10')
> > self.wait_for_console_pattern('## Loading kernel from FIT
> Image')
> > self.wait_for_console_pattern('Starting kernel ...')
> > -self.wait_for_console_pattern("systemd[1]: Hostname set to")
> >
> > @skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is
> > unstable on GitLab')
> >
> > @@ -387,15 +387,15 @@ def test_arm_ast2600_evb_sdk(self):
> > year = time.strftime("%Y")
> > self.ssh_command_output_contains('/sbin/hwclock -f
> > /dev/rtc1', year);
> >
> > -def test_aarch64_ast2700_evb_sdk_v09_01(self):
> > +def test_aarch64_ast2700_evb_sdk_v09_02(self):
> > """
> > :avocado: tags=arch:aarch64
> > :avocado: tags=machine:ast2700-evb
> > """
> >
> > image_url =
> ('https://github.com/AspeedTech-BMC/openbmc/releases/'
> > - 'download/v09.01/ast2700-default-obmc.tar.gz')
> > -image_hash =
> 'b1cc0fd73c7650d34c9c8459a243f52a91e9e27144b8608b2645ab19461d1e07'
> > + 'download/v09.02/ast2700-default-obmc.tar.gz')
> > +image_hash =
> 'ac969c2602f4e6bdb69562ff466b89ae3fe1d86e1f6797bb7969d787f82116a7'
> > image_path = self.fetch_asset(image_url,
> asset_hash=image_hash,
> > algorithm='sha256')
> > archive.extract(image_path, self.workdir) @@ -436,4 +436,5
> > @@ def test_aarch64_ast2700_evb_sdk_v09_01(self):
> >
> > self.vm.add_args('-smp', str(num_cpu))
> > self.do_test_aarch64_aspeed_sdk_start(image_dir +
> > 'image-bmc')
> > -
> > +self.wait_for_console_pattern('nodistro.0 ast2700-default ttyS12')
> > +self.ssh_connect('root', '0penBmc', False)
[PATCH v2 2/5] hw/net:ftgmac100: support 64 bits dma dram address for AST2700
ASPEED AST2700 SOC is a 64 bits quad core CPUs (Cortex-a35)
And the base address of dram is "0x4 " which
is 64bits address.
It have "Normal Priority Transmit Ring Base Address Register High(0x17C)",
"High Priority Transmit Ring Base Address Register High(0x184)" and
"Receive Ring Base Address Register High(0x18C)" to save the high part physical
address of descriptor manager.
Ex: TX descriptor manager address [34:0]
The "Normal Priority Transmit Ring Base Address Register High(0x17C)"
bits [2:0] which corresponds the bits [34:32] of the 64 bits address of
the TX ring buffer address.
The "Normal Priority Transmit Ring Base Address Register(0x20)" bits [31:0]
which corresponds the bits [31:0] of the 64 bits address
of the TX ring buffer address.
Besides, it have "TXDES 2" and "RXDES 2"
to save the high part physical address of packet buffer.
Ex: TX packet buffer address [34:0]
The "TXDES 2" bits [18:16] which corresponds the bits [34:32]
of the 64 bits address of the TX packet buffer address
and "TXDES 3" bits [31:0] which corresponds the bits [31:0]
of the 64 bits address of the TX packet buffer address.
Update TX/RX ring and descriptor data type to uint64_t
and supports TX/RX ring, descriptor and packet buffers
64 bits address for all ASPEED SOCs models.
Incrementing the version of vmstate to 2.
Introduce a new class(ftgmac100_high),
class attribute and memop handlers,
the realize handler instantiate a new memory
region and map it on top of the current one to
read/write FTGMAC100_*_HIGH regs.
Signed-off-by: Jamin Lin
---
hw/net/ftgmac100.c | 156 -
include/hw/net/ftgmac100.h | 24 --
2 files changed, 151 insertions(+), 29 deletions(-)
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 4e88430b2f..3d13f54efc 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -56,6 +56,16 @@
#define FTGMAC100_PHYDATA 0x64
#define FTGMAC100_FCR 0x68
+/*
+ * FTGMAC100 High registers
+ *
+ * values below are offset by - FTGMAC100_HIGH_OFFSET from datasheet
+ * because its memory region is start at FTGMAC100_HIGH_OFFSET
+ */
+#define FTGMAC100_NPTXR_BADR_HIGH (0x17C - FTGMAC100_HIGH_OFFSET)
+#define FTGMAC100_HPTXR_BADR_HIGH (0x184 - FTGMAC100_HIGH_OFFSET)
+#define FTGMAC100_RXR_BADR_HIGH (0x18C - FTGMAC100_HIGH_OFFSET)
+
/*
* Interrupt status register & interrupt enable register
*/
@@ -165,6 +175,8 @@
#define FTGMAC100_TXDES1_TX2FIC (1 << 30)
#define FTGMAC100_TXDES1_TXIC(1 << 31)
+#define FTGMAC100_TXDES2_TXBUF_BADR_HI(x) (((x) >> 16) & 0x7)
+
/*
* Receive descriptor
*/
@@ -198,13 +210,15 @@
#define FTGMAC100_RXDES1_UDP_CHKSUM_ERR (1 << 26)
#define FTGMAC100_RXDES1_IP_CHKSUM_ERR (1 << 27)
+#define FTGMAC100_RXDES2_RXBUF_BADR_HI(x) (((x) >> 16) & 0x7)
+
/*
* Receive and transmit Buffer Descriptor
*/
typedef struct {
uint32_tdes0;
uint32_tdes1;
-uint32_tdes2;/* not used by HW */
+uint32_tdes2;/* used by HW high address */
uint32_tdes3;
} FTGMAC100Desc;
@@ -515,12 +529,14 @@ out:
return frame_size;
}
-static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
-uint32_t tx_descriptor)
+static void ftgmac100_do_tx(FTGMAC100State *s, uint64_t tx_ring,
+uint64_t tx_descriptor)
{
+FTGMAC100Class *fc = FTGMAC100_GET_CLASS(s);
int frame_size = 0;
uint8_t *ptr = s->frame;
-uint32_t addr = tx_descriptor;
+uint64_t addr = tx_descriptor;
+uint64_t buf_addr = 0;
uint32_t flags = 0;
while (1) {
@@ -559,7 +575,12 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t
tx_ring,
len = sizeof(s->frame) - frame_size;
}
-if (dma_memory_read(&address_space_memory, bd.des3,
+buf_addr = bd.des3;
+if (fc->is_dma64) {
+buf_addr = deposit64(buf_addr, 32, 32,
+ FTGMAC100_TXDES2_TXBUF_BADR_HI(bd.des2));
+}
+if (dma_memory_read(&address_space_memory, buf_addr,
ptr, len, MEMTXATTRS_UNSPECIFIED)) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to read packet @
0x%x\n",
__func__, bd.des3);
@@ -726,9 +747,9 @@ static uint64_t ftgmac100_read(void *opaque, hwaddr addr,
unsigned size)
case FTGMAC100_MATH1:
return s->math[1];
case FTGMAC100_RXR_BADR:
-return s->rx_ring;
+return extract64(s->rx_ring, 0, 32);
case FTGMAC100_NPTXR_BADR:
-return s->tx_ring;
+return extract64(s->tx_ring, 0, 32);
case FTGMAC100_ITC:
return s->itc;
case FTGMAC100_DBLAC:
@@ -799,9 +820,8 @@ static void ftgmac100_write(void *opaque, hwaddr addr,
HWADDR_PRIx "\n", __func__, value);
return;
}
-
-s->rx_ring = value;
-s->rx_descr
Re: [PATCH v45 1/3] hw/sd/sdcard: Remove leftover comment about removed 'spi' Property
On Wed, 03 Jul 2024 11:59, Philippe Mathieu-Daudé wrote:
Commit c3287c0f70 ("hw/sd: Introduce a "sd-card" SPI variant
model") removed the 'spi' property. Remove the comment left
over.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 4
1 file changed, 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b158402704..53767beaf8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2473,10 +2473,6 @@ static Property sd_properties[] = {
DEFINE_PROP_UINT8("spec_version", SDState,
spec_version, SD_PHY_SPECv2_00_VERS),
DEFINE_PROP_DRIVE("drive", SDState, blk),
-/* We do not model the chip select pin, so allow the board to select
- * whether card should be in SSI or MMC/SD mode. It is also up to the
- * board to ensure that ssi transfers only occur when the chip select
- * is asserted. */
DEFINE_PROP_END_OF_LIST()
};
--
2.41.0
Reviewed-by: Manos Pitsidianakis
Re: [PATCH v10 11/12] hw/pci: Convert rom_bar into OnOffAuto
On Wed, 3 Jul 2024, Michael S. Tsirkin wrote:
On Wed, Jul 03, 2024 at 04:15:23AM +0200, BALATON Zoltan wrote:
On Tue, 2 Jul 2024, Michael S. Tsirkin wrote:
On Thu, Jun 27, 2024 at 03:08:00PM +0900, Akihiko Odaki wrote:
rom_bar is tristate but was defined as uint32_t so convert it into
OnOffAuto.
Signed-off-by: Akihiko Odaki
Commit log should explain why this is an improvement,
not just what's done.
diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
index e17bb50789ad..35c6c8e28493 100644
--- a/docs/igd-assign.txt
+++ b/docs/igd-assign.txt
@@ -35,7 +35,7 @@ IGD has two different modes for assignment using vfio-pci:
ISA/LPC bridge device (vfio-pci-igd-lpc-bridge) on the root bus at
PCI address 1f.0.
* The IGD device must have a VGA ROM, either provided via the romfile
- option or loaded automatically through vfio (standard). rombar=0
+ option or loaded automatically through vfio (standard). rombar=off
will disable legacy mode support.
* Hotplug of the IGD device is not supported.
* The IGD device must be a SandyBridge or newer model device.
...
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 39dae72497e0..0e920ed0691a 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -33,7 +33,7 @@
* execution as noticed with the BCM 57810 card for lack of a
* more better way to handle such issues.
* The user can still override by specifying a romfile or
- * rombar=1.
+ * rombar=on.
* Please see https://bugs.launchpad.net/qemu/+bug/1284874
* for an analysis of the 57810 card hang. When adding
* a new vendor id/device id combination below, please also add
So we are apparently breaking a bunch of users who followed
documentation to the dot. Why is this a good idea?
On/off is clearer than 1/0. But isn't 1/0 a synonym for on/off so previous
command lines would still work?
Regards,
BALATON Zoltan
I see nothing in code that would make it so:
const QEnumLookup OnOffAuto_lookup = {
.array = (const char *const[]) {
[ON_OFF_AUTO_AUTO] = "auto",
[ON_OFF_AUTO_ON] = "on",
[ON_OFF_AUTO_OFF] = "off",
},
.size = ON_OFF_AUTO__MAX
};
I also tried with an existing property:
$ ./qemu-system-x86_64 -device intel-hda,msi=0
qemu-system-x86_64: -device intel-hda,msi=0: Parameter 'msi' does not accept
value '0'
Then it was probably bit properties that also accept 0/1, on/off,
true/false. Maybe similar aliases could be added to on/off/auto?
In any case when I first saw rombar I thought it would set the BAR of the
ROM so wondered why it's 1 and not 5 or 6 or an offset. So on/off is
clearer in this case.
Regards,
BALATON Zoltan
Re: [PATCH 4/4] python: enable testing for 3.13
John Snow writes: > Python 3.13 is in beta and Fedora 41 is preparing to make it the default > system interpreter; enable testing for it. > > (In the event problems develop prior to release, it should only impact > the check-python-tox job, which is not run by default and is allowed to > fail.) > > Signed-off-by: John Snow Reviewed-by: Alex Bennée Tested-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH 2/4] python: Do not use pylint 3.2.4 with python 3.8
John Snow writes: > There is a bug in this version, > see: https://github.com/pylint-dev/pylint/issues/9751 > > Signed-off-by: John Snow Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH 1/4] python: linter changes for pylint 3.x
John Snow writes: > New bleeding edge versions, new nits to iron out. This addresses the > 'check-python-tox' optional GitLab test, while 'check-python-minreqs' > saw no regressions, since it's frozen on an older version of pylint. > > Fixes: > qemu/machine/machine.py:345:52: E0606: Possibly using variable 'sock' before > assignment (possibly-used-before-assignment) > qemu/utils/qemu_ga_client.py:168:4: R1711: Useless return at end of function > or method (useless-return) > > Signed-off-by: John Snow Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
[PATCH v8 2/3] backends: Initial support for SPDM socket support
From: Huai-Cheng Kuo SPDM enables authentication, attestation and key exchange to assist in providing infrastructure security enablement. It's a standard published by the DMTF [1]. SPDM supports multiple transports, including PCIe DOE and MCTP. This patch adds support to QEMU to connect to an external SPDM instance. SPDM support can be added to any QEMU device by exposing a TCP socket to a SPDM server. The server can then implement the SPDM decoding/encoding support, generally using libspdm [2]. This is similar to how the current TPM implementation works and means that the heavy lifting of setting up certificate chains, capabilities, measurements and complex crypto can be done outside QEMU by a well supported and tested library. 1: https://www.dmtf.org/standards/SPDM 2: https://github.com/DMTF/libspdm Signed-off-by: Huai-Cheng Kuo Signed-off-by: Chris Browy Co-developed-by: Jonathan Cameron Signed-off-by: Jonathan Cameron [ Changes by WM - Bug fixes from testing ] Signed-off-by: Wilfred Mallawa [ Changes by AF: - Convert to be more QEMU-ified - Move to backends as it isn't PCIe specific ] Signed-off-by: Alistair Francis --- MAINTAINERS | 6 + include/sysemu/spdm-socket.h | 74 backends/spdm-socket.c | 216 +++ backends/Kconfig | 4 + backends/meson.build | 2 + 5 files changed, 302 insertions(+) create mode 100644 include/sysemu/spdm-socket.h create mode 100644 backends/spdm-socket.c diff --git a/MAINTAINERS b/MAINTAINERS index 6725913c8b..c76a0cfe12 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3397,6 +3397,12 @@ F: tests/qtest/*tpm* F: docs/specs/tpm.rst T: git https://github.com/stefanberger/qemu-tpm.git tpm-next +SPDM +M: Alistair Francis +S: Maintained +F: backends/spdm-socket.c +F: include/sysemu/spdm-socket.h + Checkpatch S: Odd Fixes F: scripts/checkpatch.pl diff --git a/include/sysemu/spdm-socket.h b/include/sysemu/spdm-socket.h new file mode 100644 index 00..5d8bd9aa4e --- /dev/null +++ b/include/sysemu/spdm-socket.h @@ -0,0 +1,74 @@ +/* + * QEMU SPDM socket support + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef SPDM_REQUESTER_H +#define SPDM_REQUESTER_H + +/** + * spdm_socket_connect: connect to an external SPDM socket + * @port: port to connect to + * @errp: error object handle + * + * This will connect to an external SPDM socket server. On error + * it will return -1 and errp will be set. On success this function + * will return the socket number. + */ +int spdm_socket_connect(uint16_t port, Error **errp); + +/** + * spdm_socket_rsp: send and receive a message to a SPDM server + * @socket: socket returned from spdm_socket_connect() + * @transport_type: SPDM_SOCKET_TRANSPORT_TYPE_* macro + * @req: request buffer + * @req_len: request buffer length + * @rsp: response buffer + * @rsp_len: response buffer length + * + * Send platform data to a SPDM server on socket and then receive + * a response. + */ +uint32_t spdm_socket_rsp(const int socket, uint32_t transport_type, + void *req, uint32_t req_len, + void *rsp, uint32_t rsp_len); + +/** + * spdm_socket_close: send a shutdown command to the server + * @socket: socket returned from spdm_socket_connect() + * @transport_type: SPDM_SOCKET_TRANSPORT_TYPE_* macro + * + * This will issue a shutdown command to the server. + */ +void spdm_socket_close(const int socket, uint32_t transport_type); + +#define SPDM_SOCKET_COMMAND_NORMAL0x0001 +#define SPDM_SOCKET_COMMAND_OOB_ENCAP_KEY_UPDATE 0x8001 +#define SPDM_SOCKET_COMMAND_CONTINUE 0xFFFD +#define SPDM_SOCKET_COMMAND_SHUTDOWN 0xFFFE +#define SPDM_SOCKET_COMMAND_UNKOWN0x +#define SPDM_SOCKET_COMMAND_TEST 0xDEAD + +#define SPDM_SOCKET_TRANSPORT_TYPE_MCTP 0x01 +#define SPDM_SOCKET_TRANSPORT_TYPE_PCI_D
[PATCH v8 3/3] hw/nvme: Add SPDM over DOE support
From: Wilfred Mallawa Setup Data Object Exchange (DOE) as an extended capability for the NVME controller and connect SPDM to it (CMA) to it. Signed-off-by: Wilfred Mallawa Signed-off-by: Alistair Francis Reviewed-by: Jonathan Cameron Acked-by: Klaus Jensen --- docs/specs/index.rst| 1 + docs/specs/spdm.rst | 134 include/hw/pci/pci_device.h | 7 ++ include/hw/pci/pcie_doe.h | 3 + hw/nvme/ctrl.c | 62 + 5 files changed, 207 insertions(+) create mode 100644 docs/specs/spdm.rst diff --git a/docs/specs/index.rst b/docs/specs/index.rst index 1484e3e760..e2d907959a 100644 --- a/docs/specs/index.rst +++ b/docs/specs/index.rst @@ -29,6 +29,7 @@ guest hardware that is specific to QEMU. edu ivshmem-spec pvpanic + spdm standard-vga virt-ctlr vmcoreinfo diff --git a/docs/specs/spdm.rst b/docs/specs/spdm.rst new file mode 100644 index 00..f7de080ff0 --- /dev/null +++ b/docs/specs/spdm.rst @@ -0,0 +1,134 @@ +== +QEMU Security Protocols and Data Models (SPDM) Support +== + +SPDM enables authentication, attestation and key exchange to assist in +providing infrastructure security enablement. It's a standard published +by the `DMTF`_. + +QEMU supports connecting to a SPDM responder implementation. This allows an +external application to emulate the SPDM responder logic for an SPDM device. + +Setting up a SPDM server + + +When using QEMU with SPDM devices QEMU will connect to a server which +implements the SPDM functionality. + +SPDM-Utils +-- + +You can use `SPDM Utils`_ to emulate a responder. This is the simplest method. + +SPDM-Utils is a Linux applications to manage, test and develop devices +supporting DMTF Security Protocol and Data Model (SPDM). It is written in Rust +and utilises libspdm. + +To use SPDM-Utils you will need to do the following steps. Details are included +in the SPDM-Utils README. + + 1. `Build libspdm`_ + 2. `Build SPDM Utils`_ + 3. `Run it as a server`_ + +spdm-emu + + +You can use `spdm emu`_ to model the +SPDM responder. + +.. code-block:: shell + +$ cd spdm-emu +$ git submodule init; git submodule update --recursive +$ mkdir build; cd build +$ cmake -DARCH=x64 -DTOOLCHAIN=GCC -DTARGET=Debug -DCRYPTO=openssl .. +$ make -j32 +$ make copy_sample_key # Build certificates, required for SPDM authentication. + +It is worth noting that the certificates should be in compliance with +PCIe r6.1 sec 6.31.3. This means you will need to add the following to +openssl.cnf + +.. code-block:: + +subjectAltName = otherName:2.23.147;UTF8:Vendor=1b36:Device=0010:CC=010802:REV=02:SSVID=1af4:SSID=1100 +2.23.147 = ASN1:OID:2.23.147 + +and then manually regenerate some certificates with: + +.. code-block:: shell + +$ openssl req -nodes -newkey ec:param.pem -keyout end_responder.key \ +-out end_responder.req -sha384 -batch \ +-subj "/CN=DMTF libspdm ECP384 responder cert" + +$ openssl x509 -req -in end_responder.req -out end_responder.cert \ +-CA inter.cert -CAkey inter.key -sha384 -days 3650 -set_serial 3 \ +-extensions v3_end -extfile ../openssl.cnf + +$ openssl asn1parse -in end_responder.cert -out end_responder.cert.der + +$ cat ca.cert.der inter.cert.der end_responder.cert.der > bundle_responder.certchain.der + +You can use SPDM-Utils instead as it will generate the correct certificates +automatically. + +The responder can then be launched with + +.. code-block:: shell + +$ cd bin +$ ./spdm_responder_emu --trans PCI_DOE + +Connecting an SPDM NVMe device +== + +Once a SPDM server is running we can start QEMU and connect to the server. + +For an NVMe device first let's setup a block we can use + +.. code-block:: shell + +$ cd qemu-spdm/linux/image +$ dd if=/dev/zero of=blknvme bs=1M count=2096 # 2GB NNMe Drive + +Then you can add this to your QEMU command line: + +.. code-block:: shell + +-drive file=blknvme,if=none,id=mynvme,format=raw \ +-device nvme,drive=mynvme,serial=deadbeef,spdm_port=2323 + +At which point QEMU will try to connect to the SPDM server. + +Note that if using x64-64 you will want to use the q35 machine instead +of the default. So the entire QEMU command might look like this + +.. code-block:: shell + +qemu-system-x86_64 -M q35 \ +--kernel bzImage \ +-drive file=rootfs.ext2,if=virtio,format=raw \ +-append "root=/dev/vda console=ttyS0" \ +-net none -nographic \ +-drive file=blknvme,if=none,id=mynvme,format=raw \ +-device nvme,drive=mynvme,serial=deadbeef,spdm_port=2323 + +.. _DMTF: + https://www.dmtf.org/standards/SPDM + +.. _SPDM Utils: + https://github.com/westerndigitalcorporation/spdm-utils + +.. _spdm emu: + https://github.com/
[PATCH v8 1/3] hw/pci: Add all Data Object Types defined in PCIe r6.0
Add all of the defined protocols/features from the PCIe-SIG r6.0 "Table 6-32 PCI-SIG defined Data Object Types (Vendor ID = 0001h)" table. Signed-off-by: Alistair Francis Reviewed-by: Jonathan Cameron Reviewed-by: Wilfred Mallawa --- include/hw/pci/pcie_doe.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/hw/pci/pcie_doe.h b/include/hw/pci/pcie_doe.h index 87dc17dcef..15d94661f9 100644 --- a/include/hw/pci/pcie_doe.h +++ b/include/hw/pci/pcie_doe.h @@ -46,6 +46,8 @@ REG32(PCI_DOE_CAP_STATUS, 0) /* PCI-SIG defined Data Object Types - r6.0 Table 6-32 */ #define PCI_SIG_DOE_DISCOVERY 0x00 +#define PCI_SIG_DOE_CMA 0x01 +#define PCI_SIG_DOE_SECURED_CMA 0x02 #define PCI_DOE_DW_SIZE_MAX (1 << 18) #define PCI_DOE_PROTOCOL_NUM_MAX256 -- 2.45.2
[PATCH v8 0/3] Initial support for SPDM Responders
The Security Protocol and Data Model (SPDM) Specification defines messages, data objects, and sequences for performing message exchanges over a variety of transport and physical media. - https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.3.0.pdf SPDM currently supports PCIe DOE and MCTP transports, but it can be extended to support others in the future. This series adds support to QEMU to connect to an external SPDM instance. SPDM support can be added to any QEMU device by exposing a TCP socket to a SPDM server. The server can then implement the SPDM decoding/encoding support, generally using libspdm [1]. This is similar to how the current TPM implementation works and means that the heavy lifting of setting up certificate chains, capabilities, measurements and complex crypto can be done outside QEMU by a well supported and tested library. This series implements socket support and exposes SPDM for a NVMe device. 1: https://github.com/DMTF/libspdm v8: - Fixup i386 failures (thanks to Wilfred) - Passes CI on GitLab: https://gitlab.com/alistair23/qemu/-/tree/mainline/alistair/spdm-socket.next?ref_type=heads v7: - Fixup checkpatch failures - Fixup test failures - Rename port name to be clearer v6: - Add documentation to public functions - Rename socket variable to spdm_socket - Don't override errp - Correctly return false from nvme_init_pci() on error v5: - Update MAINTAINERS v4: - Rebase v3: - Spelling fixes - Support for SPDM-Utils v2: - Add cover letter - A few code fixes based on comments - Document SPDM-Utils - A few tweaks and clarifications to the documentation Alistair Francis (1): hw/pci: Add all Data Object Types defined in PCIe r6.0 Huai-Cheng Kuo (1): backends: Initial support for SPDM socket support Wilfred Mallawa (1): hw/nvme: Add SPDM over DOE support MAINTAINERS | 6 + docs/specs/index.rst | 1 + docs/specs/spdm.rst | 134 ++ include/hw/pci/pci_device.h | 7 ++ include/hw/pci/pcie_doe.h| 5 + include/sysemu/spdm-socket.h | 74 backends/spdm-socket.c | 216 +++ hw/nvme/ctrl.c | 62 ++ backends/Kconfig | 4 + backends/meson.build | 2 + 10 files changed, 511 insertions(+) create mode 100644 docs/specs/spdm.rst create mode 100644 include/sysemu/spdm-socket.h create mode 100644 backends/spdm-socket.c -- 2.45.2
Re: [PATCH v2 5/5] test/avocado/machine_aspeed.py: update to test network for AST2700
On 7/3/24 10:16 AM, Jamin Lin wrote:
Update a test case to test network connection via ssh and
changes to test Aspeed OpenBMC SDK v09.02 for AST2700.
ASPEED fixed TX mask issue from linux/drivers/ftgmac100.c.
It is required to use ASPEED SDK image since v09.02
for AST2700 QEMU network testing.
A test image is downloaded from the ASPEED Forked OpenBMC GitHub
release repository :
https://github.com/AspeedTech-BMC/openbmc/releases/
Test command:
```
cd build
pyvenv/bin/avocado run
../qemu/tests/avocado/machine_aspeed.py:AST2x00MachineSDK.test_aarch64_ast2700_evb_sdk_v09_02
```
Signed-off-by: Jamin Lin
Could you please split the patch ? The change of SDK should be
a standalone patch.
Thanks,
C.
---
tests/avocado/machine_aspeed.py | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/tests/avocado/machine_aspeed.py b/tests/avocado/machine_aspeed.py
index 3a20644fb2..855da805ae 100644
--- a/tests/avocado/machine_aspeed.py
+++ b/tests/avocado/machine_aspeed.py
@@ -313,14 +313,14 @@ def do_test_arm_aspeed_sdk_start(self, image):
def do_test_aarch64_aspeed_sdk_start(self, image):
self.vm.set_console()
-self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw')
+self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
+ '-net', 'nic', '-net',
'user,hostfwd=:127.0.0.1:0-:22')
self.vm.launch()
self.wait_for_console_pattern('U-Boot 2023.10')
self.wait_for_console_pattern('## Loading kernel from FIT Image')
self.wait_for_console_pattern('Starting kernel ...')
-self.wait_for_console_pattern("systemd[1]: Hostname set to")
@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on GitLab')
@@ -387,15 +387,15 @@ def test_arm_ast2600_evb_sdk(self):
year = time.strftime("%Y")
self.ssh_command_output_contains('/sbin/hwclock -f /dev/rtc1', year);
-def test_aarch64_ast2700_evb_sdk_v09_01(self):
+def test_aarch64_ast2700_evb_sdk_v09_02(self):
"""
:avocado: tags=arch:aarch64
:avocado: tags=machine:ast2700-evb
"""
image_url = ('https://github.com/AspeedTech-BMC/openbmc/releases/'
- 'download/v09.01/ast2700-default-obmc.tar.gz')
-image_hash =
'b1cc0fd73c7650d34c9c8459a243f52a91e9e27144b8608b2645ab19461d1e07'
+ 'download/v09.02/ast2700-default-obmc.tar.gz')
+image_hash =
'ac969c2602f4e6bdb69562ff466b89ae3fe1d86e1f6797bb7969d787f82116a7'
image_path = self.fetch_asset(image_url, asset_hash=image_hash,
algorithm='sha256')
archive.extract(image_path, self.workdir)
@@ -436,4 +436,5 @@ def test_aarch64_ast2700_evb_sdk_v09_01(self):
self.vm.add_args('-smp', str(num_cpu))
self.do_test_aarch64_aspeed_sdk_start(image_dir + 'image-bmc')
-
+self.wait_for_console_pattern('nodistro.0 ast2700-default ttyS12')
+self.ssh_connect('root', '0penBmc', False)
Re: [PATCH v2 4/5] hw/block: m25p80: support quad mode for w25q01jvq
On 7/3/24 10:16 AM, Jamin Lin wrote:
According to the w25q01jv datasheet at page 16,
it is required to set QE bit in "Status Register 2".
Besides, users are able to utilize "Write Status Register 1(0x01)"
command to set QE bit in "Status Register 2" and
utilize "Read Status Register 2(0x35)" command to get the QE bit status.
To support quad mode for w25q01jvq, update collecting data needed
2 bytes for WRSR command in decode_new_cmd function and
verify QE bit at the second byte of collecting data bit 2
in complete_collecting_data.
Update RDCR_EQIO command to set bit 2 of return data
if quad mode enable in decode_new_cmd.
Signed-off-by: Troy Lee
Signed-off-by: Jamin Lin
Reviewed-by: Cédric Le Goater
Thanks,
C.
---
hw/block/m25p80.c | 16
1 file changed, 16 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8dec134832..9e99107b42 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -416,6 +416,7 @@ typedef enum {
/*
* Micron: 0x35 - enable QPI
* Spansion: 0x35 - read control register
+ * Winbond: 0x35 - quad enable
*/
RDCR_EQIO = 0x35,
RSTQIO = 0xf5,
@@ -798,6 +799,11 @@ static void complete_collecting_data(Flash *s)
s->four_bytes_address_mode = extract32(s->data[1], 5, 1);
}
break;
+case MAN_WINBOND:
+if (s->len > 1) {
+s->quad_enable = !!(s->data[1] & 0x02);
+}
+break;
default:
break;
}
@@ -1254,6 +1260,10 @@ static void decode_new_cmd(Flash *s, uint32_t value)
s->needed_bytes = 2;
s->state = STATE_COLLECTING_VAR_LEN_DATA;
break;
+case MAN_WINBOND:
+s->needed_bytes = 2;
+s->state = STATE_COLLECTING_VAR_LEN_DATA;
+break;
default:
s->needed_bytes = 1;
s->state = STATE_COLLECTING_DATA;
@@ -1431,6 +1441,12 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case MAN_MACRONIX:
s->quad_enable = true;
break;
+case MAN_WINBOND:
+s->data[0] = (!!s->quad_enable) << 1;
+s->pos = 0;
+s->len = 1;
+s->state = STATE_READING_DATA;
+break;
default:
break;
}
Re: [PATCH v2 1/5] hw/net:ftgmac100: update memory region size to 0x200
On 7/3/24 10:16 AM, Jamin Lin wrote:
According to the datasheet of ASPEED SOCs,
one MAC controller owns 128KB of register space for AST2500.
However, one MAC controller only owns 64KB of register space for AST2600
and AST2700.
It set the memory region size 128KB and it occupied another
controllers Address Spaces.
Currently, the ftgmac100 model use 0x100 register space.
To support DMA 64 bits dram address and new future mode(ftgmac100_high) which
have "Normal Priority Transmit Ring Base Address Register High(0x17C)",
"High Priority Transmit Ring Base Address Register High(0x184)" and
"Receive Ring Base Address Register High(0x18C)" to save the high part physical
address of descriptor manager.
Update memory region size to 0x200.
Signed-off-by: Jamin Lin
---
hw/net/ftgmac100.c | 2 +-
include/hw/net/ftgmac100.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 25e4c0cd5b..4e88430b2f 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -1108,7 +1108,7 @@ static void ftgmac100_realize(DeviceState *dev, Error
**errp)
}
memory_region_init_io(&s->iomem, OBJECT(dev), &ftgmac100_ops, s,
- TYPE_FTGMAC100, 0x2000);
+ TYPE_FTGMAC100, FTGMAC100_NR_REGS);
sysbus_init_mmio(sbd, &s->iomem);
sysbus_init_irq(sbd, &s->irq);
qemu_macaddr_default_if_unset(&s->conf.macaddr);
diff --git a/include/hw/net/ftgmac100.h b/include/hw/net/ftgmac100.h
index 765d1538a4..5a970676da 100644
--- a/include/hw/net/ftgmac100.h
+++ b/include/hw/net/ftgmac100.h
@@ -14,6 +14,8 @@
#define TYPE_FTGMAC100 "ftgmac100"
OBJECT_DECLARE_SIMPLE_TYPE(FTGMAC100State, FTGMAC100)
+#define FTGMAC100_NR_REGS 0x200
Since this value will size a memory region, I think the define name should
be changed to FTGMAC100_{MEM,REGION,MMIO}_SIZE. What ever you prefer.
Thanks,
C.
+
#include "hw/sysbus.h"
#include "net/net.h"
[PATCH v45 3/3] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Keep it in a dedicated
'vendor_data[]' array.
Signed-off-by: Philippe Mathieu-Daudé
---
v43: Do not re-use VMSTATE_UNUSED_V (danpb)
v44: Use subsection (Luc)
v45: Remove APP_READ_BLOCK/APP_WRITE_BLOCK macros
---
hw/sd/sd.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a08a452d81..5d58937dd4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -153,6 +153,8 @@ struct SDState {
uint32_t data_offset;
size_t data_size;
uint8_t data[512];
+uint8_t vendor_data[512];
+
qemu_irq readonly_cb;
qemu_irq inserted_cb;
QEMUTimer *ocr_power_timer;
@@ -719,6 +721,7 @@ static void sd_reset(DeviceState *dev)
sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
sd->wp_group_bits = sect;
sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
+memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
memset(sd->function_group, 0, sizeof(sd->function_group));
sd->erase_start = INVALID_ADDRESS;
sd->erase_end = INVALID_ADDRESS;
@@ -793,6 +796,16 @@ static const VMStateDescription sd_ocr_vmstate = {
},
};
+static const VMStateDescription sd_vendordata_vmstate = {
+.name = "sd-card/vendor_data-state",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (const VMStateField[]) {
+VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
+VMSTATE_END_OF_LIST()
+},
+};
+
static int sd_vmstate_pre_load(void *opaque)
{
SDState *sd = opaque;
@@ -840,6 +853,7 @@ static const VMStateDescription sd_vmstate = {
},
.subsections = (const VMStateDescription * const []) {
&sd_ocr_vmstate,
+&sd_vendordata_vmstate,
NULL
},
};
@@ -902,9 +916,6 @@ static void sd_blk_write(SDState *sd, uint64_t addr,
uint32_t len)
}
}
-#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len)
-#define APP_WRITE_BLOCK(a, len)
-
static void sd_erase(SDState *sd)
{
uint64_t erase_start = sd->erase_start;
@@ -2187,9 +2198,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 56: /* CMD56: GEN_CMD */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
-APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
+sd->vendor_data[sd->data_offset++] = value;
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
}
break;
@@ -2261,12 +2271,11 @@ uint8_t sd_read_byte(SDState *sd)
break;
case 56: /* CMD56: GEN_CMD */
-if (sd->data_offset == 0)
-APP_READ_BLOCK(sd->data_start, sd->blk_len);
-ret = sd->data[sd->data_offset ++];
+ret = sd->vendor_data[sd->data_offset++];
-if (sd->data_offset >= sd->blk_len)
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
+}
break;
default:
--
2.41.0
[PATCH v45 2/3] hw/sd/sdcard: Use spec v3.01 by default
Recent SDHCI expect cards to support the v3.01 spec
to negociate lower I/O voltage. Select it by default.
Versioned machine types with a version of 9.0 or
earlier retain the old default (spec v2.00).
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Cédric Le Goater
---
v43: update versioned machines (danpb)
---
hw/core/machine.c | 1 +
hw/sd/sd.c| 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 655d75c21f..4377f943d5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -38,6 +38,7 @@ GlobalProperty hw_compat_9_0[] = {
{"arm-cpu", "backcompat-cntfrq", "true" },
{"scsi-disk-base", "migrate-emulated-scsi-request", "false" },
{"vfio-pci", "skip-vsc-check", "false" },
+{"sd-card", "spec_version", "2" },
};
const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 53767beaf8..a08a452d81 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2471,7 +2471,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
static Property sd_properties[] = {
DEFINE_PROP_UINT8("spec_version", SDState,
- spec_version, SD_PHY_SPECv2_00_VERS),
+ spec_version, SD_PHY_SPECv3_01_VERS),
DEFINE_PROP_DRIVE("drive", SDState, blk),
DEFINE_PROP_END_OF_LIST()
};
--
2.41.0
[PATCH v45 1/3] hw/sd/sdcard: Remove leftover comment about removed 'spi' Property
Commit c3287c0f70 ("hw/sd: Introduce a "sd-card" SPI variant
model") removed the 'spi' property. Remove the comment left
over.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 4
1 file changed, 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b158402704..53767beaf8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2473,10 +2473,6 @@ static Property sd_properties[] = {
DEFINE_PROP_UINT8("spec_version", SDState,
spec_version, SD_PHY_SPECv2_00_VERS),
DEFINE_PROP_DRIVE("drive", SDState, blk),
-/* We do not model the chip select pin, so allow the board to select
- * whether card should be in SSI or MMC/SD mode. It is also up to the
- * board to ensure that ssi transfers only occur when the chip select
- * is asserted. */
DEFINE_PROP_END_OF_LIST()
};
--
2.41.0
[PATCH v45 0/3] hw/sd/sdcard: Cleanups before adding eMMC support
(patches from v42 already reviewed not reposted) - Addressed review comments from Daniel & Luc wrt migration - Remove old comment Philippe Mathieu-Daudé (3): hw/sd/sdcard: Remove leftover comment about removed 'spi' Property hw/sd/sdcard: Use spec v3.01 by default hw/sd/sdcard: Do not store vendor data on block drive (CMD56) hw/core/machine.c | 1 + hw/sd/sd.c| 35 --- 2 files changed, 21 insertions(+), 15 deletions(-) -- 2.41.0
Re: [PATCH v44 2/3] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
On 3/7/24 10:51, Philippe Mathieu-Daudé wrote:
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Keep it in a dedicated
'vendor_data[]' array.
Signed-off-by: Philippe Mathieu-Daudé
---
v43: Do not re-use VMSTATE_UNUSED_V (danpb)
v44: Use subsection (Luc)
---
hw/sd/sd.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 808dc1cea6..000b923c73 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -153,6 +153,8 @@ struct SDState {
uint32_t data_offset;
size_t data_size;
uint8_t data[512];
+uint8_t vendor_data[512];
+
qemu_irq readonly_cb;
qemu_irq inserted_cb;
QEMUTimer *ocr_power_timer;
@@ -719,6 +721,7 @@ static void sd_reset(DeviceState *dev)
sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
sd->wp_group_bits = sect;
sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
+memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
memset(sd->function_group, 0, sizeof(sd->function_group));
sd->erase_start = INVALID_ADDRESS;
sd->erase_end = INVALID_ADDRESS;
@@ -793,6 +796,16 @@ static const VMStateDescription sd_ocr_vmstate = {
},
};
+static const VMStateDescription sd_vendordata_vmstate = {
+.name = "sd-card/vendor_data-state",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (const VMStateField[]) {
+VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
+VMSTATE_END_OF_LIST()
+},
+};
+
static int sd_vmstate_pre_load(void *opaque)
{
SDState *sd = opaque;
@@ -840,6 +853,7 @@ static const VMStateDescription sd_vmstate = {
},
.subsections = (const VMStateDescription * const []) {
&sd_ocr_vmstate,
+&sd_vendordata_vmstate,
NULL
},
};
Sigh, forgot to squash:
-- >8 --
@@ -919,3 +918,0 @@ static void sd_blk_write(SDState *sd, uint64_t addr,
uint32_t len)
-#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len)
-#define APP_WRITE_BLOCK(a, len)
-
---
@@ -2187,9 +2201,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 56: /* CMD56: GEN_CMD */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
-APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
+sd->vendor_data[sd->data_offset++] = value;
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
}
break;
@@ -2261,12 +2274,11 @@ uint8_t sd_read_byte(SDState *sd)
break;
case 56: /* CMD56: GEN_CMD */
-if (sd->data_offset == 0)
-APP_READ_BLOCK(sd->data_start, sd->blk_len);
-ret = sd->data[sd->data_offset ++];
+ret = sd->vendor_data[sd->data_offset++];
-if (sd->data_offset >= sd->blk_len)
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
+}
break;
default:
[PATCH] hw/ufs: Fix mcq register range determination logic
The function ufs_is_mcq_reg() only evaluated the range of the
mcq_op_reg offset, which is defined as a constant.
Therefore, it was possible for ufs_is_mcq_reg() to return true
despite ufs device is configured to not support the mcq.
This could cause ufs_mmio_read()/ufs_mmio_write() to overflow the
buffer. So fix it.
Fixes: 5c079578d2e4 ("hw/ufs: Add support MCQ of UFSHCI 4.0")
Signed-off-by: Jeuk Kim
---
hw/ufs/ufs.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index 683fff5840..cf0edd281c 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -57,7 +57,13 @@ static inline uint64_t ufs_reg_size(UfsHc *u)
static inline bool ufs_is_mcq_reg(UfsHc *u, uint64_t addr, unsigned size)
{
-uint64_t mcq_reg_addr = ufs_mcq_reg_addr(u, 0);
+uint64_t mcq_reg_addr;
+
+if (!u->params.mcq) {
+return false;
+}
+
+mcq_reg_addr = ufs_mcq_reg_addr(u, 0);
return (addr >= mcq_reg_addr &&
addr + size <= mcq_reg_addr + sizeof(u->mcq_reg));
}
--
2.34.1
[PATCH v44 1/3] hw/sd/sdcard: Use spec v3.01 by default
Recent SDHCI expect cards to support the v3.01 spec
to negociate lower I/O voltage. Select it by default.
Versioned machine types with a version of 9.0 or
earlier retain the old default (spec v2.00).
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Cédric Le Goater
---
v43: update versioned machines (danpb)
---
hw/core/machine.c | 1 +
hw/sd/sd.c| 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 655d75c21f..4377f943d5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -38,6 +38,7 @@ GlobalProperty hw_compat_9_0[] = {
{"arm-cpu", "backcompat-cntfrq", "true" },
{"scsi-disk-base", "migrate-emulated-scsi-request", "false" },
{"vfio-pci", "skip-vsc-check", "false" },
+{"sd-card", "spec_version", "2" },
};
const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b158402704..808dc1cea6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2471,7 +2471,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
static Property sd_properties[] = {
DEFINE_PROP_UINT8("spec_version", SDState,
- spec_version, SD_PHY_SPECv2_00_VERS),
+ spec_version, SD_PHY_SPECv3_01_VERS),
DEFINE_PROP_DRIVE("drive", SDState, blk),
/* We do not model the chip select pin, so allow the board to select
* whether card should be in SSI or MMC/SD mode. It is also up to the
--
2.41.0
[PATCH v44 2/3] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Keep it in a dedicated
'vendor_data[]' array.
Signed-off-by: Philippe Mathieu-Daudé
---
v43: Do not re-use VMSTATE_UNUSED_V (danpb)
v44: Use subsection (Luc)
---
hw/sd/sd.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 808dc1cea6..000b923c73 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -153,6 +153,8 @@ struct SDState {
uint32_t data_offset;
size_t data_size;
uint8_t data[512];
+uint8_t vendor_data[512];
+
qemu_irq readonly_cb;
qemu_irq inserted_cb;
QEMUTimer *ocr_power_timer;
@@ -719,6 +721,7 @@ static void sd_reset(DeviceState *dev)
sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
sd->wp_group_bits = sect;
sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
+memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
memset(sd->function_group, 0, sizeof(sd->function_group));
sd->erase_start = INVALID_ADDRESS;
sd->erase_end = INVALID_ADDRESS;
@@ -793,6 +796,16 @@ static const VMStateDescription sd_ocr_vmstate = {
},
};
+static const VMStateDescription sd_vendordata_vmstate = {
+.name = "sd-card/vendor_data-state",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (const VMStateField[]) {
+VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
+VMSTATE_END_OF_LIST()
+},
+};
+
static int sd_vmstate_pre_load(void *opaque)
{
SDState *sd = opaque;
@@ -840,6 +853,7 @@ static const VMStateDescription sd_vmstate = {
},
.subsections = (const VMStateDescription * const []) {
&sd_ocr_vmstate,
+&sd_vendordata_vmstate,
NULL
},
};
@@ -2187,9 +2201,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 56: /* CMD56: GEN_CMD */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
-APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
+sd->vendor_data[sd->data_offset++] = value;
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
}
break;
@@ -2261,12 +2274,11 @@ uint8_t sd_read_byte(SDState *sd)
break;
case 56: /* CMD56: GEN_CMD */
-if (sd->data_offset == 0)
-APP_READ_BLOCK(sd->data_start, sd->blk_len);
-ret = sd->data[sd->data_offset ++];
+ret = sd->vendor_data[sd->data_offset++];
-if (sd->data_offset >= sd->blk_len)
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
+}
break;
default:
--
2.41.0
[PATCH v44 3/3] hw/sd/sdcard: Remove leftover comment about removed 'spi' Property
Commit c3287c0f70 ("hw/sd: Introduce a "sd-card" SPI variant
model") removed the 'spi' property. Remove the comment left
over.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 4
1 file changed, 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 000b923c73..904da440ba 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2485,10 +2485,6 @@ static Property sd_properties[] = {
DEFINE_PROP_UINT8("spec_version", SDState,
spec_version, SD_PHY_SPECv3_01_VERS),
DEFINE_PROP_DRIVE("drive", SDState, blk),
-/* We do not model the chip select pin, so allow the board to select
- * whether card should be in SSI or MMC/SD mode. It is also up to the
- * board to ensure that ssi transfers only occur when the chip select
- * is asserted. */
DEFINE_PROP_END_OF_LIST()
};
--
2.41.0
[PATCH v44 0/3] hw/sd/sdcard: Cleanups before adding eMMC support
(patches from v42 already reviewed not reposted) - Addressed review comments from Daniel & Luc wrt migration - Remove old comment Philippe Mathieu-Daudé (3): hw/sd/sdcard: Use spec v3.01 by default hw/sd/sdcard: Do not store vendor data on block drive (CMD56) hw/sd/sdcard: Remove leftover comment about removed 'spi' Property hw/core/machine.c | 1 + hw/sd/sd.c| 32 2 files changed, 21 insertions(+), 12 deletions(-) -- 2.41.0
Re: [PATCH v43 2/2] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
On 3/7/24 09:39, Luc Michel wrote:
On 18:10 Tue 02 Jul , Philippe Mathieu-Daudé wrote:
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Keep it in a dedicated
'vendor_data[]' array.
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
v43: Do not re-use VMSTATE_UNUSED_V (danpb)
---
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 808dc1cea6..418ccb14a4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -153,6 +153,8 @@ struct SDState {
uint32_t data_offset;
size_t data_size;
uint8_t data[512];
+uint8_t vendor_data[512];
+
qemu_irq readonly_cb;
qemu_irq inserted_cb;
QEMUTimer *ocr_power_timer;
@@ -719,6 +721,7 @@ static void sd_reset(DeviceState *dev)
sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
sd->wp_group_bits = sect;
sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
+memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
memset(sd->function_group, 0, sizeof(sd->function_group));
sd->erase_start = INVALID_ADDRESS;
sd->erase_end = INVALID_ADDRESS;
@@ -835,6 +838,7 @@ static const VMStateDescription sd_vmstate = {
VMSTATE_UINT32(data_offset, SDState),
VMSTATE_UINT8_ARRAY(data, SDState, 512),
VMSTATE_UNUSED_V(1, 512),
+VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
Don't you need to bump the VMState version then?
Indeed. I'll add a subsection which is the recommended way:
https://www.qemu.org/docs/master/devel/migration/main.html#subsections
The most common structure change is adding new data, e.g. when
adding a newer form of device, or adding that state that you
previously forgot to migrate. This is best solved using a subsection.
Thanks,
Phil.
