RE: [PATCH v3 00/10] add generic vDPA device support
> -Original Message- > From: Stefano Garzarella [mailto:sgarz...@redhat.com] > Sent: Monday, March 28, 2022 11:12 PM > To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.) > > Cc: stefa...@redhat.com; m...@redhat.com; coh...@redhat.com; > pbonz...@redhat.com; Gonglei (Arei) ; Yechuan > ; Huangzhichao ; > qemu-devel@nongnu.org > Subject: Re: [PATCH v3 00/10] add generic vDPA device support > > On Sat, Mar 19, 2022 at 03:20:02PM +0800, Longpeng(Mike) wrote: > >From: Longpeng > > > >Hi guys, > > > >With the generic vDPA device, QEMU won't need to touch the device > >types any more, such like vfio. > > > >We can use the generic vDPA device as follow: > > -device vhost-vdpa-device-pci,vdpa-dev=/dev/vhost-vdpa-X > > > >I've done some simple tests on Huawei's offloading card (net, 0.95) > >and vdpa_sim_blk (1.0); > > The whole seems almost okay, I left you some comments, but for the next > version maybe it's better to reorganize the series differently. > > Instead of adding vdpa-dev and vdpa-dev-pci incrementally, with stub > functions and untestable parts, I would make a patch that adds vdpa-dev > in full and then one that adds vdpa-dev-pci. Ok, will do in next version. > The first 2 patches on the other hand are okay, maybe patch 2 won't be > needed since I think we will sync the headers after the 7.0 release, but > is better to keep it for now to simplify the review. > Yes, that's also my original intent. Thanks. > Thanks, > Stefano
RE: [PATCH v3 05/10] vdpa-dev: implement the realize interface
> -Original Message-
> From: Stefano Garzarella [mailto:sgarz...@redhat.com]
> Sent: Monday, March 28, 2022 11:00 PM
> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
>
> Cc: Stefan Hajnoczi ; Michael Tsirkin ;
> Cornelia Huck ; Paolo Bonzini ;
> Gonglei (Arei) ; Yechuan ;
> Huangzhichao ; qemu devel list
>
> Subject: Re: [PATCH v3 05/10] vdpa-dev: implement the realize interface
>
> On Sat, Mar 19, 2022 at 03:20:07PM +0800, Longpeng(Mike) wrote:
> >From: Longpeng
> >
[snap]
> > static void vhost_vdpa_device_unrealize(DeviceState *dev)
> >@@ -66,6 +197,7 @@ static void vhost_vdpa_device_set_status(VirtIODevice
> >*vdev,
> uint8_t status)
> > static Property vhost_vdpa_device_properties[] = {
> > DEFINE_PROP_STRING("vdpa-dev", VhostVdpaDevice, vdpa_dev),
> > DEFINE_PROP_INT32("vdpa-dev-fd", VhostVdpaDevice, vdpa_dev_fd, -1),
>
> Other vhost devices use the `vhostfd` property, maybe we should use the
> same name.
>
> If we go for this change, then maybe we also need to change `vdpa-dev`
> to `vhostpath` or something like that
>
Make sense.
I prefer to use 'vhostdev' for the file path, just like the usage in Netdev:
-netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0,id=vhostvdpa1
> Thanks,
> Stefano
>
> >+DEFINE_PROP_UINT16("queue-size", VhostVdpaDevice, queue_size, 0),
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> >diff --git a/include/hw/virtio/vdpa-dev.h b/include/hw/virtio/vdpa-dev.h
> >index 476bda0873..cf11abd0f7 100644
> >--- a/include/hw/virtio/vdpa-dev.h
> >+++ b/include/hw/virtio/vdpa-dev.h
> >@@ -28,6 +28,16 @@ struct VhostVdpaDevice {
> > char *vdpa_dev;
> > int vdpa_dev_fd;
> > int32_t bootindex;
> >+uint32_t vdev_id;
> >+uint32_t num_queues;
> >+struct vhost_dev dev;
> >+struct vhost_vdpa vdpa;
> >+VirtQueue **virtqs;
> >+uint8_t *config;
> >+int config_size;
> >+uint16_t queue_size;
> >+bool started;
> >+int (*post_init)(VhostVdpaDevice *v, Error **errp);
> > };
> >
> > #endif
> >--
> >2.23.0
> >
RE: [PATCH v3 05/10] vdpa-dev: implement the realize interface
> -Original Message-
> From: Stefano Garzarella [mailto:sgarz...@redhat.com]
> Sent: Monday, March 28, 2022 11:00 PM
> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
>
> Cc: Stefan Hajnoczi ; Michael Tsirkin ;
> Cornelia Huck ; Paolo Bonzini ;
> Gonglei (Arei) ; Yechuan ;
> Huangzhichao ; qemu devel list
>
> Subject: Re: [PATCH v3 05/10] vdpa-dev: implement the realize interface
>
> On Sat, Mar 19, 2022 at 03:20:07PM +0800, Longpeng(Mike) wrote:
> >From: Longpeng
> >
> >Implements the .realize interface.
> >
> >Signed-off-by: Longpeng
> >---
> > hw/virtio/vdpa-dev-pci.c | 18 -
> > hw/virtio/vdpa-dev.c | 132 +++
> > include/hw/virtio/vdpa-dev.h | 10 +++
> > 3 files changed, 159 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/virtio/vdpa-dev-pci.c b/hw/virtio/vdpa-dev-pci.c
> >index 9eb590ce8c..31bd17353a 100644
> >--- a/hw/virtio/vdpa-dev-pci.c
> >+++ b/hw/virtio/vdpa-dev-pci.c
> >@@ -51,10 +51,26 @@ static Property vhost_vdpa_device_pci_properties[] = {
> > DEFINE_PROP_END_OF_LIST(),
> > };
> >
> >+static int vhost_vdpa_device_pci_post_init(VhostVdpaDevice *v, Error **errp)
> >+{
> >+VhostVdpaDevicePCI *dev = container_of(v, VhostVdpaDevicePCI, vdev);
> >+VirtIOPCIProxy *vpci_dev = >parent_obj;
> >+
> >+vpci_dev->class_code = virtio_pci_get_class_id(v->vdev_id);
> >+vpci_dev->trans_devid = virtio_pci_get_trans_devid(v->vdev_id);
> >+/* one for config vector */
> >+vpci_dev->nvectors = v->num_queues + 1;
> >+
> >+return 0;
> >+}
> >+
> > static void
> > vhost_vdpa_device_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> > {
> >-return;
> >+VhostVdpaDevicePCI *dev = VHOST_VDPA_DEVICE_PCI(vpci_dev);
> >+
> >+dev->vdev.post_init = vhost_vdpa_device_pci_post_init;
> >+qdev_realize(DEVICE(>vdev), BUS(_dev->bus), errp);
> > }
> >
> > static void vhost_vdpa_device_pci_class_init(ObjectClass *klass, void *data)
> >diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
> >index 993cbc7d11..4defe6c33d 100644
> >--- a/hw/virtio/vdpa-dev.c
> >+++ b/hw/virtio/vdpa-dev.c
> >@@ -29,9 +29,140 @@
> > #include "sysemu/sysemu.h"
> > #include "sysemu/runstate.h"
> >
> >+static void
> >+vhost_vdpa_device_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq)
> >+{
> >+/* Nothing to do */
> >+}
> >+
> >+static uint32_t
> >+vhost_vdpa_device_get_u32(int fd, unsigned long int cmd, Error **errp)
> >+{
> >+uint32_t val = (uint32_t)-1;
> >+
> >+if (ioctl(fd, cmd, ) < 0) {
> >+error_setg(errp, "vhost-vdpa-device: cmd 0x%lx failed: %s",
> >+ cmd, strerror(errno));
> >+}
> >+
> >+return val;
> >+}
> >+
> > static void vhost_vdpa_device_realize(DeviceState *dev, Error **errp)
> > {
> >+VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> >+VhostVdpaDevice *v = VHOST_VDPA_DEVICE(vdev);
> >+uint32_t max_queue_size;
> >+struct vhost_virtqueue *vqs;
> >+int i, ret;
> >+
> >+if (!v->vdpa_dev || v->vdpa_dev_fd == -1) {
> ^
> Should we use && operator here?
>
> I can't start QEMU, how did you test this series?
>
Oh! I changed to && operator in my local environment when I test this series
but forgot to include it.
I'll fix it in the next version.
> $ ./qemu-system-x86_64 -m 512M -smp 2 -M q35,accel=kvm \
>...
>-device vhost-vdpa-device-pci,vdpa-dev=/dev/vhost-vdpa-0
> qemu-system-x86_64: -device
> vhost-vdpa-device-pci,vdpa-dev=/dev/vhost-vdpa-0: both vpda-dev and
> vpda-dev-fd are missing
>
> >+error_setg(errp, "both vpda-dev and vpda-dev-fd are missing");
>
> Typo s/vpda/vdpa
>
OK.
> >+return;
> >+}
> >+
> >+if (v->vdpa_dev && v->vdpa_dev_fd != -1) {
> >+error_setg(errp, "both vpda-dev and vpda-dev-fd are set");
>
> Ditto
>
OK.
> >+return;
> >+}
> >+
> >+if (v->vdpa_dev_fd == -1) {
> >+v->vdpa_dev_fd = qemu_open(v->vdpa_dev, O_RDWR, errp);
> >+if (*errp) {
> >+return;
> >+}
> >+}
> >+v->vdpa.device_fd = v->vdpa_dev_fd;
> >+
> >+v->vdev_id = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
> >+ VHOST_VDPA_GET_DEVICE_ID, errp);
> >+if (*errp) {
> >+goto out;
> >+}
> >+
> >+max_queue_size = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
> >+ VHOST_VDPA_GET_VRING_NUM,
> >errp);
> >+if (*errp) {
> >+goto out;
> >+}
> >+
> >+if (v->queue_size > max_queue_size) {
> >+error_setg(errp, "vhost-vdpa-device: invalid queue_size: %u
> (max:%u)",
> >+ v->queue_size, max_queue_size);
> >+goto out;
> >+} else if (!v->queue_size) {
> >+v->queue_size = max_queue_size;
> >+}
> >+
> >+v->num_queues = vhost_vdpa_device_get_u32(v->vdpa_dev_fd,
> >+ VHOST_VDPA_GET_VQS_COUNT,
> >errp);
> >+if (*errp) {
> >+
Re: [PATCH V2 4/4] intel-iommu: PASID support
在 2022/3/28 下午4:45, Yi Liu 写道:
On 2022/3/21 13:54, Jason Wang wrote:
This patch introduce ECAP_PASID via "x-pasid-mode". Based on the
existing support for scalable mode, we need to implement the following
missing parts:
1) tag VTDAddressSpace with PASID and support IOMMU/DMA translation
with PASID
should it be tagging with bdf+pasid?
The problem is BDF is programmable by the guest. So we may end up
duplicated BDFs. That's why the code uses struct PCIBus.
2) tag IOTLB with PASID
3) PASID cache and its flush
4) Fault recording with PASID
For simplicity:
1) PASID cache is not implemented so we can simply implement the PASID
cache flush as a nop.
2) Fault recording with PASID is not supported, NFR is not changed.
I think this doesn't work for passthrough device. So need to fail the
qemu if user tries to expose such a vIOMMU togather with passthroug
device.
Ok, I think I can simply fail the vIOMMU notifier registering to block
both vhost and VFIO.
Thanks
All of the above is not mandatory and could be implemented in the
future.
Note that though PASID based IOMMU translation is ready but no device
can issue PASID DMA right now. In this case, PCI_NO_PASID is used as
PASID to identify the address w/ PASID. vtd_find_add_as() has been
extended to provision address space with PASID which could be utilized
by the future extension of PCI core to allow device model to use PASID
based DMA translation.
This feature would be useful for:
1) prototyping PASID support for devices like virtio
2) future vPASID work
3) future PRS and vSVA work
Signed-off-by: Jason Wang
---
hw/i386/intel_iommu.c | 357 +
hw/i386/intel_iommu_internal.h | 14 +-
hw/i386/trace-events | 2 +
include/hw/i386/intel_iommu.h | 7 +-
include/hw/pci/pci_bus.h | 2 +
5 files changed, 296 insertions(+), 86 deletions(-)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 82787f9850..13447fda16 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -58,6 +58,14 @@
struct vtd_as_key {
PCIBus *bus;
uint8_t devfn;
+ uint32_t pasid;
+};
+
+struct vtd_iotlb_key {
+ uint16_t sid;
+ uint32_t pasid;
+ uint64_t gfn;
+ uint32_t level;
};
static void vtd_address_space_refresh_all(IntelIOMMUState *s);
@@ -199,14 +207,24 @@ static inline gboolean
vtd_as_has_map_notifier(VTDAddressSpace *as)
}
/* GHashTable functions */
-static gboolean vtd_uint64_equal(gconstpointer v1, gconstpointer v2)
+static gboolean vtd_iotlb_equal(gconstpointer v1, gconstpointer v2)
{
- return *((const uint64_t *)v1) == *((const uint64_t *)v2);
+ const struct vtd_iotlb_key *key1 = v1;
+ const struct vtd_iotlb_key *key2 = v2;
+
+ return key1->sid == key2->sid &&
+ key1->pasid == key2->pasid &&
+ key1->level == key2->level &&
+ key1->gfn == key2->gfn;
}
-static guint vtd_uint64_hash(gconstpointer v)
+static guint vtd_iotlb_hash(gconstpointer v)
{
- return (guint)*(const uint64_t *)v;
+ const struct vtd_iotlb_key *key = v;
+
+ return key->gfn | ((key->sid) << VTD_IOTLB_SID_SHIFT) |
+ (key->level) << VTD_IOTLB_LVL_SHIFT |
+ (key->pasid) << VTD_IOTLB_PASID_SHIFT;
}
static gboolean vtd_as_equal(gconstpointer v1, gconstpointer v2)
@@ -214,7 +232,8 @@ static gboolean vtd_as_equal(gconstpointer v1,
gconstpointer v2)
const struct vtd_as_key *key1 = v1;
const struct vtd_as_key *key2 = v2;
- return (key1->bus == key2->bus) && (key1->devfn == key2->devfn);
+ return (key1->bus == key2->bus) && (key1->devfn == key2->devfn) &&
+ (key1->pasid == key2->pasid);
}
static inline uint16_t vtd_make_source_id(uint8_t bus_num,
uint8_t devfn)
@@ -306,13 +325,6 @@ static void vtd_reset_caches(IntelIOMMUState *s)
vtd_iommu_unlock(s);
}
-static uint64_t vtd_get_iotlb_key(uint64_t gfn, uint16_t source_id,
- uint32_t level)
-{
- return gfn | ((uint64_t)(source_id) << VTD_IOTLB_SID_SHIFT) |
- ((uint64_t)(level) << VTD_IOTLB_LVL_SHIFT);
-}
-
static uint64_t vtd_get_iotlb_gfn(hwaddr addr, uint32_t level)
{
return (addr & vtd_slpt_level_page_mask(level)) >>
VTD_PAGE_SHIFT_4K;
@@ -320,15 +332,17 @@ static uint64_t vtd_get_iotlb_gfn(hwaddr addr,
uint32_t level)
/* Must be called with IOMMU lock held */
static VTDIOTLBEntry *vtd_lookup_iotlb(IntelIOMMUState *s, uint16_t
source_id,
- hwaddr addr)
+ hwaddr addr, uint32_t pasid)
{
+ struct vtd_iotlb_key key;
VTDIOTLBEntry *entry;
- uint64_t key;
int level;
for (level = VTD_SL_PT_LEVEL; level < VTD_SL_PML4_LEVEL;
level++) {
- key = vtd_get_iotlb_key(vtd_get_iotlb_gfn(addr, level),
- source_id, level);
+ key.gfn = vtd_get_iotlb_gfn(addr, level);
+ key.level =
Re: [PATCH V2 1/4] intel-iommu: don't warn guest errors when getting rid2pasid entry
在 2022/3/28 下午4:53, Yi Liu 写道:
On 2022/3/28 10:27, Jason Wang wrote:
On Thu, Mar 24, 2022 at 4:21 PM Tian, Kevin
wrote:
From: Jason Wang
Sent: Monday, March 21, 2022 1:54 PM
We use to warn on wrong rid2pasid entry. But this error could be
triggered by the guest and could happens during initialization. So
let's don't warn in this case.
Signed-off-by: Jason Wang
---
hw/i386/intel_iommu.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 874d01c162..90964b201c 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -1554,8 +1554,10 @@ static bool vtd_dev_pt_enabled(IntelIOMMUState
*s, VTDContextEntry *ce)
if (s->root_scalable) {
ret = vtd_ce_get_rid2pasid_entry(s, ce, );
if (ret) {
- error_report_once("%s: vtd_ce_get_rid2pasid_entry
error: %"PRId32,
- __func__, ret);
+ /*
+ * This error is guest triggerable. We should assumt PT
+ * not enabled for safety.
+ */
suppose a VT-d fault should be queued in this case besides returning
false:
SPD.1: A hardware attempt to access the scalable-mode PASID-directory
entry referenced through the PASIDDIRPTR field in scalable-mode
context-entry resulted in an error
SPT.1: A hardware attempt to access a scalable-mode PASID-table entry
referenced through the SMPTBLPTR field in a scalable-mode
PASID-directory
entry resulted in an error.
Probably, but this issue is not introduced in this patch. We can fix
it on top if necessary.
agreed.
Currently the implementation of vtd_ce_get_rid2pasid_entry() is also
problematic. According to VT-d spec, RID2PASID field is effective only
when ecap.rps is true otherwise PASID#0 is used for RID2PASID. I didn't
see ecap.rps is set, neither is it checked in that function. It
works possibly
just because Linux currently programs 0 to RID2PASID...
This seems to be another issue since the introduction of scalable mode.
yes. this is not introduced in this series. The current scalable mode
vIOMMU support was following 3.0 spec, while RPS is added in 3.1. Needs
to be fixed.
Interesting, so this is more complicated when dealing with migration
compatibility. So what I suggest is probably something like:
-device intel-iommu,version=$version
Then we can maintain migration compatibility correctly. For 3.0 we can
go without RPS and 3.1 and above we need to implement RPS.
Since most of the advanced features has not been implemented, we may
probably start just from 3.4 (assuming it's the latest version). And all
of the following effort should be done for 3.4 in order to productize it.
Thanks
Thanks
return false;
}
return (VTD_PE_GET_TYPE() == VTD_SM_PASID_ENTRY_PT);
--
2.25.1
Re: [PATCH V2 4/4] intel-iommu: PASID support
On Mon, Mar 28, 2022 at 3:03 PM Tian, Kevin wrote:
>
> > From: Jason Wang
> > Sent: Monday, March 21, 2022 1:54 PM
> >
> > +/*
> > + * vtd-spec v3.4 3.14:
> > + *
> > + * """
> > + * Requests-with-PASID with input address in range 0xFEEx_ are
> > + * translated normally like any other request-with-PASID through
> > + * DMA-remapping hardware. However, if such a request is processed
> > + * using pass-through translation, it will be blocked as described
> > + * in the paragraph below.
>
> While PASID+PT is blocked as described in the below paragraph, the
> paragraph itself applies to all situations:
>
> 1) PT + noPASID
> 2) translation + noPASID
> 3) PT + PASID
> 4) translation + PASID
>
> because...
>
> > + *
> > + * Software must not program paging-structure entries to remap any
> > + * address to the interrupt address range. Untranslated requests
> > + * and translation requests that result in an address in the
> > + * interrupt range will be blocked with condition code LGN.4 or
> > + * SGN.8.
>
> ... if you look at the definition of LGN.4 or SGN.8:
>
> LGN.4: When legacy mode (RTADDR_REG.TTM=00b) is enabled, hardware
> detected an output address (i.e. address after remapping) in the
> interrupt address range (0xFEEx_). For Translated requests and
> requests with pass-through translation type (TT=10), the output
> address is the same as the address in the request
>
> The last sentence in the first paragraph above just highlights the fact that
> when input address of PT is in interrupt range then it is blocked by LGN.4
> or SGN.8 due to output address also in interrupt range.
>
> > + * """
> > + *
> > + * We enable per as memory region (iommu_ir_fault) for catching
> > + * the tranlsation for interrupt range through PASID + PT.
> > + */
> > +if (pt && as->pasid != PCI_NO_PASID) {
> > +memory_region_set_enabled(>iommu_ir_fault, true);
> > +} else {
> > +memory_region_set_enabled(>iommu_ir_fault, false);
> > +}
> > +
>
> Given above this should be a bug fix for nopasid first and then apply it
> to pasid path too.
Actually, nopasid path patches were posted here.
https://www.mail-archive.com/qemu-devel@nongnu.org/msg867878.html
Thanks
>
> Thanks
> Kevin
>
Re: [PATCH V2 4/4] intel-iommu: PASID support
On Mon, Mar 28, 2022 at 2:47 PM Tian, Kevin wrote: > > > From: Jason Wang > > Sent: Monday, March 28, 2022 10:31 AM > > > > On Thu, Mar 24, 2022 at 4:54 PM Tian, Kevin wrote: > > > > > > > From: Jason Wang > > > > Sent: Monday, March 21, 2022 1:54 PM > > > > > > > > This patch introduce ECAP_PASID via "x-pasid-mode". Based on the > > > > existing support for scalable mode, we need to implement the following > > > > missing parts: > > > > > > > > 1) tag VTDAddressSpace with PASID and support IOMMU/DMA > > translation > > > >with PASID > > > > 2) tag IOTLB with PASID > > > > > > and invalidate desc to flush PASID iotlb, which seems missing in this > > > patch. > > > > It existed in the previous version, but it looks like it will be used > > only for the first level page table which is not supported right now. > > So I deleted the codes. > > You are right. But there is also PASID-based device TLB invalidate descriptor > which is orthogonal to 1st vs. 2nd level thing. If we don't want to break the > spec with this series then there will need a way to prevent the user from > setting both "device-iotlb" and "x-pasid-mode" together. Right, let me do it in the next version. > > > > > > > > > > 3) PASID cache and its flush > > > > 4) Fault recording with PASID > > > > > > > > For simplicity: > > > > > > > > 1) PASID cache is not implemented so we can simply implement the PASID > > > > cache flush as a nop. > > > > 2) Fault recording with PASID is not supported, NFR is not changed. > > > > > > > > All of the above is not mandatory and could be implemented in the > > > > future. > > > > > > PASID cache is optional, but fault recording with PASID is required. > > > > Any pointer in the spec to say something like this? I think sticking > > to the NFR would be sufficient. > > I didn't remember any place in spec saying that fault recording with PASID is > not required when PASID capability is exposed. Ok, but as a spec it needs to clarify what is required for each capability. > If there is certain fault > triggered by a request with PASID, we do want to report this information > upward. I tend to do it increasingly on top of this series (anyhow at least RID2PASID is introduced before this series) > > btw can you elaborate why NFR matters to PASID? It is just about the > number of fault recording register... I might be wrong, but I thought without increasing NFR we may lack sufficient room for reporting PASID. > > > > > > I'm fine with adding it incrementally but want to clarify the concept > > > first. > > > > Yes, that's the plan. > > > > I have one open which requires your input. > > While incrementally enabling things does be a common practice, one worry > is whether we want to create too many control knobs in the staging process > to cause confusion to the end user. It should be fine as long as we use the "x-" prefix which will be finally removed. > > Earlier when Yi proposed Qemu changes for guest SVA [1] he aimed for a > coarse-grained knob design: > -- > Intel VT-d 3.0 introduces scalable mode, and it has a bunch of capabilities > related to scalable mode translation, thus there are multiple combinations. > While this vIOMMU implementation wants simplify it for user by providing > typical combinations. User could config it by "x-scalable-mode" option. The > usage is as below: > "-device intel-iommu,x-scalable-mode=["legacy"|"modern"]" > > - "legacy": gives support for SL page table > - "modern": gives support for FL page table, pasid, virtual command > - if not configured, means no scalable mode support, if not proper >configured, will throw error > -- > > Which way do you prefer to? > > [1] https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg02805.html My understanding is that, if we want to deploy Qemu in a production environment, we can't use the "x-" prefix. We need a full implementation of each cap. E.g -device intel-iommu,first-level=on,scalable-mode=on etc. Thanks > > Thanks > Kevin
Re: [PATCH 1/2] gdbstub: Set current_cpu for memory read write
On Mon, Mar 28, 2022 at 5:10 PM Peter Maydell wrote: > > On Mon, 28 Mar 2022 at 03:10, Bin Meng wrote: > > IMHO it's too bad to just ignore this bug forever. > > > > This is a valid use case. It's not about whether we intentionally want > > to inspect the GIC register value from gdb. The case is that when > > single stepping the source codes it triggers the core dump for no > > reason if the instructions involved contain load/store to any of the > > GIC registers. > > Huh? Single-stepping the instruction should execute it inside > QEMU, which will do the load in the usual way. That should not > be going via gdbstub reads and writes. Yes, single-stepping the instruction is executed in the vCPU context, but a gdb client sends additional commands, more than just telling QEMU to execute a single instruction. For example, the following is the sequence a gdb client sent when doing a "si": gdbstub_io_command Received: Z0,10,4 gdbstub_io_reply Sent: OK gdbstub_io_got_ack Got ACK gdbstub_io_command Received: m18c430,4 gdbstub_io_reply Sent: ff430091 gdbstub_io_got_ack Got ACK gdbstub_io_command Received: vCont;s:p1.1;c:p1.-1 gdbstub_op_stepping Stepping CPU 0 gdbstub_op_continue_cpu Continuing CPU 1 gdbstub_op_continue_cpu Continuing CPU 2 gdbstub_op_continue_cpu Continuing CPU 3 gdbstub_hit_break RUN_STATE_DEBUG gdbstub_io_reply Sent: T05thread:p01.01; gdbstub_io_got_ack Got ACK gdbstub_io_command Received: g gdbstub_io_reply Sent: 3848ed00f08fa6100300010001f930a5ec0034c41800c903 gdbstub_io_got_ack Got ACK gdbstub_io_command Received: m18c434,4 gdbstub_io_reply Sent: 00e004d1 gdbstub_io_got_ack Got ACK gdbstub_io_command Received: m18c430,4 gdbstub_io_reply Sent: ff430091 gdbstub_io_got_ack Got ACK gdbstub_io_command Received: m18c434,4 gdbstub_io_reply Sent: 00e004d1 gdbstub_io_got_ack Got ACK gdbstub_io_command Received: m18c400,40 gdbstub_io_reply Sent: ff4300d1e00300f98037005840f900a0019140f900b0009140f900e004911e7800f9fe0340f91ef9ff43009100e004d174390094bb390094 gdbstub_io_got_ack Got ACK gdbstub_io_command Received: mf901,4 Here "mf901,4" triggers the bug where 0xf901 is the GIC register. This is not something QEMU can ignore or control. The logic is inside the gdb client. Regards, Bin
Re: [RFC PATCH 0/6] softfloat 128-bit integer support
On 3/28/22 14:14, matheus.fe...@eldorado.org.br wrote: From: Matheus Ferst This RFC is a first attempt at implementing the 128-bit integer conversion routines in softfloat, as required by the xscv[su]qqp and xscvqp[su]qz instructions of PowerISA v3.1. Instead of using int128.h, int-to-float routines receive the 128-bit numbers through a pair of 64-bit values, and float-to-int conversions use a pointer to return the lower half of the result. We only need the parts128 methods, but since the difference to parts64 ones seemed minor, I included both in this patch. RFC: - Should we use struct Int128 instead of 64-bit value pairs? I think so. We have it, and it makes the interface more obvious. - I've not tested the float64 methods since the PPC instructions only use the quad-precision routines. Should we keep them in the final version? Let's not add anything that we don't have a need for. It may eventually be needed by RISC-V RV128, but we can add it then. r~
Re: [RFC PATCH v7 29/29] tests/tcg/loongarch64: Add hello/memory test in loongarch64 system
On 3/28/22 06:57, Xiaojuan Yang wrote: + .global _start + .align 16 +_start: + bl main You must at least set up the stack. This must work only via some interaction with efi bios? What we *want* is for this to run by itself on the raw virt machine. r~
Re: [RFC PATCH v7 24/29] hw/loongarch: Add default bios startup support.
On 3/28/22 06:57, Xiaojuan Yang wrote:
Signed-off-by: Xiaojuan Yang
Signed-off-by: Song Gao
---
hw/loongarch/Kconfig | 4 ++
hw/loongarch/fw_cfg.c| 33 ++
hw/loongarch/fw_cfg.h| 15 +++
hw/loongarch/loongson3.c | 76 +++-
hw/loongarch/meson.build | 1 +
include/hw/loongarch/loongarch.h | 8
6 files changed, 135 insertions(+), 2 deletions(-)
create mode 100644 hw/loongarch/fw_cfg.c
create mode 100644 hw/loongarch/fw_cfg.h
Not within this patch, but you will want to create a submodule for this bios image, as we
do for roms/edk2 etc. You will want to commit a pre-built image as well, like we do for
other targets.
+/* load the BIOS image. */
+filename = qemu_find_file(QEMU_FILE_TYPE_BIOS,
+ machine->firmware ?: LOONGSON3_BIOSNAME);
+if (filename) {
+bios_size = load_image_targphys(filename, LA_BIOS_BASE, LA_BIOS_SIZE);
+lams->fw_cfg = loongarch_fw_cfg_init(ram_size, machine);
+rom_set_fw(lams->fw_cfg);
+g_free(filename);
+} else {
+bios_size = -1;
+}
While loading the efi bios may be the most common usage, it's not efficient for simple
tests. I think you should attempt to load an elf image, and only if that fails treat it
as the firmware blob that efi builds.
Or not require a bios image if -kernel is present, or something.
At the moment one cannot run the tests/tcg/multiarch tests without
loongarch.bios.
r~
Re: [PATCH for-7.0 v5] qemu-binfmt-conf.sh: mips: allow nonzero EI_ABIVERSION, distinguish o32 and n32
On 3/29/22 07:39, Philippe Mathieu-Daudé wrote: On 28/3/22 22:49, Andreas K. Hüttel wrote: With the command line flag -mplt and a recent toolchain, ELF binaries generated by gcc can obtain EI_ABIVERSION=1, which makes, e.g., gcc three-stage bootstrap in a mips-unknown-linux-gnu qemu-user chroot fail since the binfmt-misc magic does not match anymore. Also other values are technically possible. qemu executes these binaries just fine, so relax the mask for the EI_ABIVERSION byte at offset 0x08. In addition, extend magic string to distinguish mips o32 and n32 ABI. This information is given by the EF_MIPS_ABI2 (0x20) bit in the e_flags field of the ELF header (a 4-byte value at offset 0x24 for the here applicable ELFCLASS32). See-also: ace3d65459 Signed-off-by: Andreas K. Hüttel Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: WANG Xuerui v5 changes are too different from v4 to keep these R-b tags IMO. LGTM but I'd like Xuerui to double-check the R-b stands, and an Acked-by from Laurent would make me feel safer ;) This is just a squash of v4 patches, but I manually double-checked the definitions against elf.h for that extra confidence, and they looked good. The R-b tags still stand :) -- WANG Xuerui xe...@gentoo.org Gentoo Linux developer PGP: 7C52 19E3 26A0 7311 3EA3 8806 C01F 7214 BC93 1414 OpenPGP_0xC01F7214BC931414.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 0/2] ppc: fix vcpu hotunplug leak in spapr_realize_vcpu
On Mon, Mar 28, 2022 at 09:59:16AM -0300, Daniel Henrique Barboza wrote: > Hi, > > This is a memory leak found by Valgrind when testing vcpu > hotplug/unplug in pSeries guests. > > Other vcpu hotplug/unplug leaks are still present in the common code > (one in the KVM thread loop and another in cpu_address_space via > cpu->cpu_ases) but these are already being handled by Mark Kanda and > Phillipe. Changes LGTM, but I don't see much reason to split this into two patches. They're both small, and are part of the same logical change. > > > Daniel Henrique Barboza (2): > hw/ppc/ppc.c: add cpu_ppc_tb_free() > hw/ppc: free env->tb_env in spapr_unrealize_vcpu() > > hw/ppc/ppc.c| 7 +++ > hw/ppc/spapr_cpu_core.c | 3 +++ > include/hw/ppc/ppc.h| 1 + > 3 files changed, 11 insertions(+) > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [PATCH v5 09/13] KVM: Handle page fault for private memory
On Thu, Mar 10, 2022, Chao Peng wrote:
> @@ -3890,7 +3893,59 @@ static bool kvm_arch_setup_async_pf(struct kvm_vcpu
> *vcpu, gpa_t cr2_or_gpa,
> kvm_vcpu_gfn_to_hva(vcpu, gfn), );
> }
>
> -static bool kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault
> *fault, int *r)
> +static bool kvm_vcpu_is_private_gfn(struct kvm_vcpu *vcpu, gfn_t gfn)
> +{
> + /*
> + * At this time private gfn has not been supported yet. Other patch
> + * that enables it should change this.
> + */
> + return false;
> +}
> +
> +static bool kvm_faultin_pfn_private(struct kvm_vcpu *vcpu,
> + struct kvm_page_fault *fault,
> + bool *is_private_pfn, int *r)
@is_private_pfn should be a field in @fault, not a separate parameter, and it
should be a const property set by the original caller. I would also name it
"is_private", because if KVM proceeds past this point, it will be a property of
the fault/access _and_ the pfn
I say it's a property of the fault because the below kvm_vcpu_is_private_gfn()
should instead be:
if (fault->is_private)
The kvm_vcpu_is_private_gfn() check is TDX centric. For SNP, private vs. shared
is communicated via error code. For software-only (I'm being optimistic ;-) ),
we'd probably need to track private vs. shared internally in KVM, I don't think
we'd want to force it to be a property of the gfn.
Then you can also move the fault->is_private waiver into is_page_fault_stale(),
and drop the local is_private_pfn in direct_page_fault().
> +{
> + int order;
> + unsigned int flags = 0;
> + struct kvm_memory_slot *slot = fault->slot;
> + long pfn = kvm_memfile_get_pfn(slot, fault->gfn, );
If get_lock_pfn() and thus kvm_memfile_get_pfn() returns a pure error code
instead
of multiplexing the pfn, then this can be:
bool is_private_pfn;
is_private_pfn = !!kvm_memfile_get_pfn(slot, fault->gfn, >pfn,
);
That self-documents the "pfn < 0" == shared logic.
> +
> + if (kvm_vcpu_is_private_gfn(vcpu, fault->addr >> PAGE_SHIFT)) {
> + if (pfn < 0)
> + flags |= KVM_MEMORY_EXIT_FLAG_PRIVATE;
> + else {
> + fault->pfn = pfn;
> + if (slot->flags & KVM_MEM_READONLY)
> + fault->map_writable = false;
> + else
> + fault->map_writable = true;
> +
> + if (order == 0)
> + fault->max_level = PG_LEVEL_4K;
This doesn't correctly handle order > 0, but less than the next page size, in
which
case max_level needs to be PG_LEVEL_4k. It also doesn't handle the case where
max_level > PG_LEVEL_2M.
That said, I think the proper fix is to have the get_lock_pfn() API return the
max
mapping level, not the order. KVM, and presumably any other secondary MMU that
might
use these APIs, doesn't care about the order of the struct page, KVM cares
about the
max size/level of page it can map into the guest. And similar to the previous
patch,
"order" is specific to struct page, which we are trying to avoid.
> + *is_private_pfn = true;
This is where KVM guarantees that is_private_pfn == fault->is_private.
> + *r = RET_PF_FIXED;
> + return true;
Ewww. This is super confusing. Ditto for the "*r = -1" magic number. I
totally
understand why you took this approach, it's just hard to follow because it kinda
follows the kvm_faultin_pfn() semantics, but then inverts true and false in this
one case.
I think the least awful option is to forego the helper and open code everything.
If we ever refactor kvm_faultin_pfn() to be less weird then we can maybe move
this
to a helper.
Open coding isn't too bad if you reorganize things so that the exit-to-userspace
path is a dedicated, early check. IMO, it's a lot easier to read this way, open
coded or not.
I think this is correct? "is_private_pfn" and "level" are locals, everything
else
is in @fault.
if (kvm_slot_is_private(slot)) {
is_private_pfn = !!kvm_memfile_get_pfn(slot, fault->gfn,
>pfn, );
if (fault->is_private != is_private_pfn) {
if (is_private_pfn)
kvm_memfile_put_pfn(slot, fault->pfn);
vcpu->run->exit_reason = KVM_EXIT_MEMORY_ERROR;
if (fault->is_private)
vcpu->run->memory.flags =
KVM_MEMORY_EXIT_FLAG_PRIVATE;
else
vcpu->run->memory.flags = 0;
vcpu->run->memory.padding = 0;
vcpu->run->memory.gpa = fault->gfn << PAGE_SHIFT;
vcpu->run->memory.size = PAGE_SIZE;
*r = 0;
Re: [PATCH v5 00/13] KVM: mm: fd-based approach for supporting KVM guest private memory
On Mon, Mar 28, 2022, Nakajima, Jun wrote: > > On Mar 28, 2022, at 1:16 PM, Andy Lutomirski wrote: > > > > On Thu, Mar 10, 2022 at 6:09 AM Chao Peng > > wrote: > >> > >> This is the v5 of this series which tries to implement the fd-based KVM > >> guest private memory. The patches are based on latest kvm/queue branch > >> commit: > >> > >> d5089416b7fb KVM: x86: Introduce KVM_CAP_DISABLE_QUIRKS2 > > > > Can this series be run and a VM booted without TDX? A feature like > > that might help push it forward. > > > > —Andy > > Since the userspace VMM (e.g. QEMU) loses direct access to private memory of > the VM, the guest needs to avoid using the private memory for (virtual) DMA > buffers, for example. Otherwise, it would need to use bounce buffers, i.e. we > would need changes to the VM. I think we can try that (i.e. add only bounce > buffer changes). What do you think? I would love to be able to test this series and run full-blown VMs without TDX or SEV hardware. The other option for getting test coverage is KVM selftests, which don't have an existing guest that needs to be enlightened. Vishal is doing work on that front, though I think it's still in early stages. Long term, selftests will also be great for negative testing.
Re: [PATCH v5 08/13] KVM: Use memfile_pfn_ops to obtain pfn for private pages
On Thu, Mar 10, 2022, Chao Peng wrote:
> @@ -2217,4 +2220,34 @@ static inline void kvm_handle_signal_exit(struct
> kvm_vcpu *vcpu)
> /* Max number of entries allowed for each kvm dirty ring */
> #define KVM_DIRTY_RING_MAX_ENTRIES 65536
>
> +#ifdef CONFIG_MEMFILE_NOTIFIER
> +static inline long kvm_memfile_get_pfn(struct kvm_memory_slot *slot, gfn_t
> gfn,
> +int *order)
> +{
> + pgoff_t index = gfn - slot->base_gfn +
> + (slot->private_offset >> PAGE_SHIFT);
This is broken for 32-bit kernels, where gfn_t is a 64-bit value but pgoff_t is
a
32-bit value. There's no reason to support this for 32-bit kernels, so...
The easiest fix, and likely most maintainable for other code too, would be to
add a dedicated CONFIG for private memory, and then have KVM check that for all
the memfile stuff. x86 can then select it only for 64-bit kernels, and in turn
select MEMFILE_NOTIFIER iff private memory is supported.
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index ca7b2a6a452a..ee9c8c155300 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -48,7 +48,9 @@ config KVM
select SRCU
select INTERVAL_TREE
select HAVE_KVM_PM_NOTIFIER if PM
- select MEMFILE_NOTIFIER
+ select HAVE_KVM_PRIVATE_MEM if X86_64
+ select MEMFILE_NOTIFIER if HAVE_KVM_PRIVATE_MEM
+
help
Support hosting fully virtualized guest machines using hardware
virtualization extensions. You will need a fairly recent
And in addition to replacing checks on CONFIG_MEMFILE_NOTIFIER, the probing of
whether or not KVM_MEM_PRIVATE is allowed can be:
@@ -1499,23 +1499,19 @@ static void kvm_replace_memslot(struct kvm *kvm,
}
}
-bool __weak kvm_arch_private_memory_supported(struct kvm *kvm)
-{
- return false;
-}
-
static int check_memory_region_flags(struct kvm *kvm,
const struct kvm_userspace_memory_region *mem)
{
u32 valid_flags = KVM_MEM_LOG_DIRTY_PAGES;
- if (kvm_arch_private_memory_supported(kvm))
- valid_flags |= KVM_MEM_PRIVATE;
-
#ifdef __KVM_HAVE_READONLY_MEM
valid_flags |= KVM_MEM_READONLY;
#endif
+#ifdef CONFIG_KVM_HAVE_PRIVATE_MEM
+ valid_flags |= KVM_MEM_PRIVATE;
+#endif
+
if (mem->flags & ~valid_flags)
return -EINVAL;
> +
> + return slot->pfn_ops->get_lock_pfn(file_inode(slot->private_file),
> +index, order);
In a similar vein, get_locK_pfn() shouldn't return a "long". KVM likely won't
use
these APIs on 32-bit kernels, but that may not hold true for other subsystems,
and
this code is confusing and technically wrong. The pfns for struct page squeeze
into an unsigned long because PAE support is capped at 64gb, but casting to a
signed long could result in a pfn with bit 31 set being misinterpreted as an
error.
Even returning an "unsigned long" for the pfn is wrong. It "works" for the
shmem
code because shmem deals only with struct page, but it's technically wrong,
especially
since one of the selling points of this approach is that it can work without
struct
page.
OUT params suck, but I don't see a better option than having the return value be
0/-errno, with "pfn_t *pfn" for the resolved pfn.
> +}
> +
> +static inline void kvm_memfile_put_pfn(struct kvm_memory_slot *slot,
> +kvm_pfn_t pfn)
> +{
> + slot->pfn_ops->put_unlock_pfn(pfn);
> +}
> +
> +#else
> +static inline long kvm_memfile_get_pfn(struct kvm_memory_slot *slot, gfn_t
> gfn,
> +int *order)
> +{
This should be a WARN_ON() as its usage should be guarded by a KVM_PRIVATE_MEM
check, and private memslots should be disallowed in this case.
Alternatively, it might be a good idea to #ifdef these out entirely and not
provide
stubs. That'd likely require a stub or two in arch code, but overall it might
be
less painful in the long run, e.g. would force us to more carefully consider the
touch points for private memory. Definitely not a requirement, just an idea.
Re: [PATCH for-7.0 v5] qemu-binfmt-conf.sh: mips: allow nonzero EI_ABIVERSION, distinguish o32 and n32
On 28/3/22 22:49, Andreas K. Hüttel wrote: With the command line flag -mplt and a recent toolchain, ELF binaries generated by gcc can obtain EI_ABIVERSION=1, which makes, e.g., gcc three-stage bootstrap in a mips-unknown-linux-gnu qemu-user chroot fail since the binfmt-misc magic does not match anymore. Also other values are technically possible. qemu executes these binaries just fine, so relax the mask for the EI_ABIVERSION byte at offset 0x08. In addition, extend magic string to distinguish mips o32 and n32 ABI. This information is given by the EF_MIPS_ABI2 (0x20) bit in the e_flags field of the ELF header (a 4-byte value at offset 0x24 for the here applicable ELFCLASS32). See-also: ace3d65459 Signed-off-by: Andreas K. Hüttel Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: WANG Xuerui v5 changes are too different from v4 to keep these R-b tags IMO. LGTM but I'd like Xuerui to double-check the R-b stands, and an Acked-by from Laurent would make me feel safer ;) Cc: Laurent Vivier Cc: WANG Xuerui Cc: Richard Henderson Cc: Alex Bennee Cc: Philippe Mathieu-Daudé Closes: https://gitlab.com/qemu-project/qemu/-/issues/843 --- v5: Fully relax mask for EI_ABIVERSION for all of mips; squash patches since they touch the same lines v4: Unchanged repost of v3 v3: Add the magic extension to distinguish n32 and o32 v2: Add the same EI_ABIVERSION fix for little endian as for big endian v1: Initial version, only handling EI_ABIVERSION=1 on BE scripts/qemu-binfmt-conf.sh | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh index e9bfeb94d3..9cb723f443 100755 --- a/scripts/qemu-binfmt-conf.sh +++ b/scripts/qemu-binfmt-conf.sh @@ -60,28 +60,28 @@ m68k_family=m68k # FIXME: We could use the other endianness on a MIPS host. -mips_magic='\x7fELF\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08' -mips_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff' +mips_magic='\x7fELF\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' +mips_mask='\xff\xff\xff\xff\xff\xff\xff\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20' mips_family=mips -mipsel_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00' -mipsel_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff' +mipsel_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' +mipsel_mask='\xff\xff\xff\xff\xff\xff\xff\x00\x00\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20\x00\x00\x00' mipsel_family=mips -mipsn32_magic='\x7fELF\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08' -mipsn32_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff' +mipsn32_magic='\x7fELF\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20' +mipsn32_mask='\xff\xff\xff\xff\xff\xff\xff\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20' mipsn32_family=mips -mipsn32el_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00' -mipsn32el_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff' +mipsn32el_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20\x00\x00\x00' +mipsn32el_mask='\xff\xff\xff\xff\xff\xff\xff\x00\x00\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20\x00\x00\x00' mipsn32el_family=mips mips64_magic='\x7fELF\x02\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08' -mips64_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff' +mips64_mask='\xff\xff\xff\xff\xff\xff\xff\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff' mips64_family=mips mips64el_magic='\x7fELF\x02\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00' -mips64el_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff' +mips64el_mask='\xff\xff\xff\xff\xff\xff\xff\x00\x00\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff' mips64el_family=mips sh4_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x2a\x00'
Re: [RFC PATCH v7 21/29] Enable common virtio pci support for LoongArch
On 3/28/22 06:57, Xiaojuan Yang wrote: Signed-off-by: Xiaojuan Yang Signed-off-by: Song Gao --- softmmu/qdev-monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [RFC PATCH v7 20/29] hw/loongarch: Add irq hierarchy for the system
On 3/28/22 06:57, Xiaojuan Yang wrote:
+static struct DeviceState *ipi, *extioi;
Static variables can't be right.
These need to be somewhere else, or ...
@@ -107,12 +115,101 @@ static void loongarch_cpu_init(LoongArchCPU *la_cpu, int
cpu_num)
NULL, "iocsr_misc", IOCSR_MEM_SIZE);
memory_region_add_subregion(>system_iocsr, 0, >iocsr_mem);
+/* ipi memory region */
+ipi_addr = SMP_IPI_MAILBOX + cpu_num * 0x100;
+memory_region_add_subregion(>system_iocsr, ipi_addr,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
+cpu_num));
+/* extioi memory region */
+memory_region_add_subregion(>system_iocsr, APIC_BASE,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
+cpu_num));
... this code needs to be moved somewhere else.
+static void loongarch_irq_init(LoongArchMachineState *lams,
+ DeviceState *ipi, DeviceState *extioi)
+{
+MachineState *ms = MACHINE(lams);
+DeviceState *pch_pic, *pch_msi, *cpudev;
+
+SysBusDevice *d;
+int cpu, pin, i;
+
+for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
+cpudev = DEVICE(qemu_get_cpu(cpu));
+/* connect ipi irq to cpu irq */
+qdev_connect_gpio_out(ipi, cpu, qdev_get_gpio_in(cpudev, IRQ_IPI));
+}
+
+for (i = 0; i < EXTIOI_IRQS; i++) {
+sysbus_connect_irq(SYS_BUS_DEVICE(extioi),
+ i, qdev_get_gpio_in(extioi, i));
+}
Um... connecting extioi to itself?
I think that graphic that you used in the description of patch 15 belongs here
as a comment.
diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
new file mode 100644
index 00..bf80e99ce1
--- /dev/null
+++ b/include/hw/pci-host/ls7a.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * QEMU LoongArch CPU
+ *
+ * Copyright (c) 2021 Loongson Technology Corporation Limited
+ */
+
+#ifndef HW_LS7A_H
+#define HW_LS7A_H
+
+#include "hw/pci/pci.h"
+#include "hw/pci/pcie_host.h"
+#include "hw/pci-host/pam.h"
+#include "qemu/units.h"
+#include "qemu/range.h"
+#include "qom/object.h"
+
+#define LS7A_PCH_REG_BASE 0x1000UL
+#define LS7A_IOAPIC_REG_BASE(LS7A_PCH_REG_BASE)
+#define LS7A_PCH_MSI_ADDR_LOW 0x2FF0UL
+
+/*
+ * According to the kernel pch irq start from 64 offset
+ * 0 ~ 16 irqs used for non-pci device while 16 ~ 64 irqs
+ * used for pci device.
+ */
+#define PCH_PIC_IRQ_OFFSET 64
+#define LS7A_DEVICE_IRQS16
+#define LS7A_PCI_IRQS 48
+#endif
Why is this file in this patch?
It seems like it should be in either patch 17 or 18?
r~
Re: [RFC PATCH v7 09/29] target/loongarch: Add TLB instruction support
On 3/28/22 06:57, Xiaojuan Yang wrote:
+void helper_tlbflush(CPULoongArchState *env)
+{
+int i, index;
+
+index = FIELD_EX64(env->CSR_TLBIDX, CSR_TLBIDX, INDEX);
+
+if (index < LOONGARCH_STLB) {
+/* STLB. One line per operation */
+for (i = 0; i < 8; i++) {
+int index = i * 256 + (index % 256);
Another uninitialized use Werror that you should have seen: You're shadowing the outer
'index' variable, which means this doesn't do what you intended.
r~
Re: [RFC PATCH v7 19/29] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)
On 3/28/22 06:57, Xiaojuan Yang wrote:
+static uint64_t extioi_ipmap_enable_read(void *opaque, hwaddr addr,
+ unsigned size)
+{
+LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+uint8_t ret;
+
+switch (addr) {
+case EXTIOI_IPMAP_START ... EXTIOI_IPMAP_END - 1:
+ret = s->ipmap[addr];
+break;
+case EXTIOI_ENABLE_START ... EXTIOI_ENABLE_END - 1:
+addr -= EXTIOI_ENABLE_START;
+ret = s->enable[addr];
+break;
+default:
+break;
+}
+
+trace_loongarch_extioi_ipmap_enable_read((uint8_t)addr, ret);
+return ret;
+}
There are at least 4 instances of uninitialized use of 'ret' in this file, and this is
one. Are you compiling with --disable-werror? You must not do that.
r~
Re: [PATCH v5 00/13] KVM: mm: fd-based approach for supporting KVM guest private memory
> On Mar 28, 2022, at 1:16 PM, Andy Lutomirski wrote: > > On Thu, Mar 10, 2022 at 6:09 AM Chao Peng wrote: >> >> This is the v5 of this series which tries to implement the fd-based KVM >> guest private memory. The patches are based on latest kvm/queue branch >> commit: >> >> d5089416b7fb KVM: x86: Introduce KVM_CAP_DISABLE_QUIRKS2 > > Can this series be run and a VM booted without TDX? A feature like > that might help push it forward. > > —Andy Since the userspace VMM (e.g. QEMU) loses direct access to private memory of the VM, the guest needs to avoid using the private memory for (virtual) DMA buffers, for example. Otherwise, it would need to use bounce buffers, i.e. we would need changes to the VM. I think we can try that (i.e. add only bounce buffer changes). What do you think? Thanks, --- Jun
Re: [PATCH] memory: Make memory_region_readd_subregion() properly handle mapped aliases
On 1/2/22 11:09, David Hildenbrand wrote:
memory_region_readd_subregion() wants to readd a region by first
removing it and then readding it. For readding, it doesn't use one of
the memory_region_add_*() variants, which is why fail to re-increment the
mr->mapped_via_alias counters, resulting in the
assert(alias->mapped_via_alias >= 0) in memory_region_del_subregion()
triggering the next time we call memory_region_readd_subregion().
Fix it by using memory_region_add_subregion_common() for readding the
region.
Reported-by: Niek Linnenbank
Fixes: 5ead62185d23 ("memory: Make memory_region_is_mapped() succeed when mapped via
an alias")
Tested-by: Niek Linnenbank
Cc: Paolo Bonzini
Cc: Peter Xu
Cc: "Philippe Mathieu-Daudé"
Signed-off-by: David Hildenbrand
---
softmmu/memory.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Thanks, queued to mips-fixes.
Re: [RFC PATCH v7 19/29] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)
On 3/28/22 06:57, Xiaojuan Yang wrote:
This patch realize the EIOINTC interrupt controller.
Signed-off-by: Xiaojuan Yang
Signed-off-by: Song Gao
---
hw/intc/Kconfig| 3 +
hw/intc/loongarch_extioi.c | 408 +
hw/intc/meson.build| 1 +
hw/intc/trace-events | 11 +
hw/loongarch/Kconfig | 1 +
include/hw/intc/loongarch_extioi.h | 77 ++
6 files changed, 501 insertions(+)
create mode 100644 hw/intc/loongarch_extioi.c
create mode 100644 include/hw/intc/loongarch_extioi.h
diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 71c04c328e..28bd1f185d 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -96,3 +96,6 @@ config LOONGARCH_PCH_MSI
select MSI_NONBROKEN
bool
select UNIMP
+
+config LOONGARCH_EXTIOI
+bool
diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
new file mode 100644
index 00..af28e8d6e9
--- /dev/null
+++ b/hw/intc/loongarch_extioi.c
@@ -0,0 +1,408 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Loongson 3A5000 ext interrupt controller emulation
+ *
+ * Copyright (C) 2021 Loongson Technology Corporation Limited
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qemu/log.h"
+#include "hw/irq.h"
+#include "hw/sysbus.h"
+#include "hw/loongarch/loongarch.h"
+#include "hw/qdev-properties.h"
+#include "exec/address-spaces.h"
+#include "hw/intc/loongarch_extioi.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+static void extioi_update_irq(void *opaque, int irq_num, int level)
+{
+LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
I think this is not opaque anymore; you've already resolved it in the caller.
I think level should be 'bool'.
+uint8_t ipnum, cpu;
+unsigned long found1, found2;
+
+ipnum = s->sw_ipmap[irq_num];
+cpu = s->sw_coremap[irq_num];
+if (level == 1) {
Just if (level).
+if (test_bit(irq_num, (void *)s->enable) == false) {
This, and every other cast you're using for bitops.h functions, is wrong. You would need
to declare these bitmaps properly as 'unsigned long name[BITS_TO_LONGS(N)];'.
That said, I would definitely use uint64_t, because that matches up with the description
of these registers in the manual.
+return;
+}
+bitmap_set((void *)s->coreisr[cpu], irq_num, 1);
+found1 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
+ EXTIOI_IRQS, 0);
find_next_bit with offset=0 is find_first_bit...
+bitmap_set((void *)&(s->sw_ipisr[cpu][ipnum]), irq_num, 1);
+
+if (found1 >= EXTIOI_IRQS) {
+qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+}
... but what's the bitmap search doing? It appears to be checking that there are *no*
bits set between 0 and EXTIOI_IRQS, and then raising the irq if no bits set. That seems
wrong.
+} else {
+bitmap_clear((void *)s->coreisr[cpu], irq_num, 1);
+found1 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
+ EXTIOI_IRQS, 0);
+bitmap_clear((void *)&(s->sw_ipisr[cpu][ipnum]), irq_num, 1);
+found2 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
+ EXTIOI_IRQS, 0);
+
+if ((found1 < EXTIOI_IRQS) && (found2 >= EXTIOI_IRQS)) {
+qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+}
+}
+}
It *seems* like all of this should be
uint64_t sum = 0;
s->isr[ipnum / 64] = deposit64(s->isr[ipnum / 64], ipnum % 64, 1, level);
for (int i = 0; i < ARRAY_SIZE(s->isr); i++) {
sum |= s->isr[i] & s->ena[i];
}
qemu_set_irq(parent, sum != 0);
If that's not the case, you need many more comments.
r~
Re: [PATCH for-7.0 v2] target/mips: Fix address space range declaration on n32
On 28/3/22 05:59, WANG Xuerui wrote: This bug is probably lurking there for so long, I cannot even git-blame my way to the commit first introducing it. Anyway, because n32 is also TARGET_MIPS64, the address space range cannot be determined by looking at TARGET_MIPS64 alone. Fix this by only declaring 48-bit address spaces for n64, or the n32 user emulation will happily hand out memory ranges beyond the 31-bit limit and crash. Confirmed to make the minimal reproducing example in the linked issue behave. Closes: https://gitlab.com/qemu-project/qemu/-/issues/939 Signed-off-by: WANG Xuerui Cc: Philippe Mathieu-Daudé Cc: Aurelien Jarno Cc: Jiaxun Yang Cc: Aleksandar Rikalo Tested-by: Andreas K. Huettel Reviewed-by: Richard Henderson --- v2: - Collect tags - Make it clear this patch is for 7.0 target/mips/cpu-param.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé And queued to mips-fixes, thanks.
[PULL 4/4] ui/console: Check console before emitting GL event
From: Akihiko Odaki
Without this change, The GL output of a console overwrites the
other consoles and makes them unusable.
Signed-off-by: Akihiko Odaki
Reviewed-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20220325161216.74582-1-akihiko.od...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé
---
ui/console.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/ui/console.c b/ui/console.c
index da434ce1b2..1752f2ec88 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1886,6 +1886,9 @@ void dpy_gl_scanout_disable(QemuConsole *con)
con->scanout.kind = SCANOUT_NONE;
}
QLIST_FOREACH(dcl, >listeners, next) {
+if (con != (dcl->con ? dcl->con : active_console)) {
+continue;
+}
if (dcl->ops->dpy_gl_scanout_disable) {
dcl->ops->dpy_gl_scanout_disable(dcl);
}
@@ -1909,6 +1912,9 @@ void dpy_gl_scanout_texture(QemuConsole *con,
x, y, width, height
};
QLIST_FOREACH(dcl, >listeners, next) {
+if (con != (dcl->con ? dcl->con : active_console)) {
+continue;
+}
if (dcl->ops->dpy_gl_scanout_texture) {
dcl->ops->dpy_gl_scanout_texture(dcl, backing_id,
backing_y_0_top,
@@ -1927,6 +1933,9 @@ void dpy_gl_scanout_dmabuf(QemuConsole *con,
con->scanout.kind = SCANOUT_DMABUF;
con->scanout.dmabuf = dmabuf;
QLIST_FOREACH(dcl, >listeners, next) {
+if (con != (dcl->con ? dcl->con : active_console)) {
+continue;
+}
if (dcl->ops->dpy_gl_scanout_dmabuf) {
dcl->ops->dpy_gl_scanout_dmabuf(dcl, dmabuf);
}
@@ -1940,6 +1949,9 @@ void dpy_gl_cursor_dmabuf(QemuConsole *con, QemuDmaBuf
*dmabuf,
DisplayChangeListener *dcl;
QLIST_FOREACH(dcl, >listeners, next) {
+if (con != (dcl->con ? dcl->con : active_console)) {
+continue;
+}
if (dcl->ops->dpy_gl_cursor_dmabuf) {
dcl->ops->dpy_gl_cursor_dmabuf(dcl, dmabuf,
have_hot, hot_x, hot_y);
@@ -1954,6 +1966,9 @@ void dpy_gl_cursor_position(QemuConsole *con,
DisplayChangeListener *dcl;
QLIST_FOREACH(dcl, >listeners, next) {
+if (con != (dcl->con ? dcl->con : active_console)) {
+continue;
+}
if (dcl->ops->dpy_gl_cursor_position) {
dcl->ops->dpy_gl_cursor_position(dcl, pos_x, pos_y);
}
@@ -1967,6 +1982,9 @@ void dpy_gl_release_dmabuf(QemuConsole *con,
DisplayChangeListener *dcl;
QLIST_FOREACH(dcl, >listeners, next) {
+if (con != (dcl->con ? dcl->con : active_console)) {
+continue;
+}
if (dcl->ops->dpy_gl_release_dmabuf) {
dcl->ops->dpy_gl_release_dmabuf(dcl, dmabuf);
}
@@ -1983,6 +2001,9 @@ void dpy_gl_update(QemuConsole *con,
graphic_hw_gl_block(con, true);
QLIST_FOREACH(dcl, >listeners, next) {
+if (con != (dcl->con ? dcl->con : active_console)) {
+continue;
+}
if (dcl->ops->dpy_gl_update) {
dcl->ops->dpy_gl_update(dcl, x, y, w, h);
}
--
2.35.1
[PULL 0/4] Darwin patches for 2022-03-29
From: Philippe Mathieu-Daudé The following changes since commit 27fc9f365d6f60ff86c2e2be57289bb47a2be882: Merge tag 'pull-ppc-20220326' of https://github.com/legoater/qemu into staging (2022-03-28 10:16:33 +0100) are available in the Git repository at: https://github.com/philmd/qemu.git tags/darwin-20220329 for you to fetch changes up to a4fd374364d4e23e0861273aaf7ff2ebddd57a17: ui/console: Check console before emitting GL event (2022-03-29 00:19:37 +0200) Darwin patches - UI fixes Akihiko Odaki (2): ui/cocoa: Respect left-command-key option ui/console: Check console before emitting GL event Philippe Mathieu-Daudé (2): gitattributes: Cover Objective-C source files qemu/main-loop: Disable block backend global state assertion on Cocoa .gitattributes | 1 + include/qemu/main-loop.h | 13 + ui/cocoa.m | 3 ++- ui/console.c | 21 + 4 files changed, 37 insertions(+), 1 deletion(-) -- 2.35.1
[PULL 2/4] qemu/main-loop: Disable block backend global state assertion on Cocoa
From: Philippe Mathieu-Daudé
Since commit 0439c5a462 ("block/block-backend.c: assertions for
block-backend") QEMU crashes when using Cocoa on Darwin hosts.
Example on macOS:
$ qemu-system-i386
Assertion failed: (qemu_in_main_thread()), function blk_all_next, file
block-backend.c, line 552.
Abort trap: 6
Looking with lldb:
Assertion failed: (qemu_in_main_thread()), function blk_all_next, file
block-backend.c, line 552.
Process 76914 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = hit program assert
frame #4: 0x00010057c2d4 qemu-system-i386`blk_all_next.cold.1
at block-backend.c:552:5 [opt]
549*/
550 BlockBackend *blk_all_next(BlockBackend *blk)
551 {
--> 552 GLOBAL_STATE_CODE();
553 return blk ? QTAILQ_NEXT(blk, link)
554 : QTAILQ_FIRST(_backends);
555 }
Target 1: (qemu-system-i386) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = hit program assert
frame #0: 0x0001908c99b8 libsystem_kernel.dylib`__pthread_kill + 8
frame #1: 0x0001908fceb0 libsystem_pthread.dylib`pthread_kill + 288
frame #2: 0x00019083a314 libsystem_c.dylib`abort + 164
frame #3: 0x00019083972c libsystem_c.dylib`__assert_rtn + 300
* frame #4: 0x00010057c2d4 qemu-system-i386`blk_all_next.cold.1 at
block-backend.c:552:5 [opt]
frame #5: 0x0001003c00b4
qemu-system-i386`blk_all_next(blk=) at block-backend.c:552:5 [opt]
frame #6: 0x0001003d8f04
qemu-system-i386`qmp_query_block(errp=0x) at qapi.c:591:16 [opt]
frame #7: 0x00010003ab0c qemu-system-i386`main [inlined]
addRemovableDevicesMenuItems at cocoa.m:1756:21 [opt]
frame #8: 0x00010003ab04 qemu-system-i386`main(argc=,
argv=) at cocoa.m:1980:5 [opt]
frame #9: 0x0001012690f4 dyld`start + 520
As we are in passed release 7.0 hard freeze, disable the block
backend assertion which, while being valuable during development,
is not helpful to users. We'll restore this assertion immediately
once 7.0 is released and work on a fix.
Suggested-by: Akihiko Odaki
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Akihiko Odaki
Reviewed-by: Peter Maydell
Message-Id: <20220325183707.85733-1-philippe.mathieu.da...@gmail.com>
---
include/qemu/main-loop.h | 13 +
1 file changed, 13 insertions(+)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 7a4d6a0920..89bd9edefb 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -270,10 +270,23 @@ bool qemu_mutex_iothread_locked(void);
bool qemu_in_main_thread(void);
/* Mark and check that the function is part of the global state API. */
+#ifdef CONFIG_COCOA
+/*
+ * When using the Cocoa UI, addRemovableDevicesMenuItems() is called from
+ * a thread different from the QEMU main thread and can not take the BQL,
+ * triggering this assertions in the block layer (commit 0439c5a462).
+ * As the Cocoa fix is not trivial, disable this assertion for the v7.0.0
+ * release (when using Cocoa); we will restore it immediately after the
+ * release.
+ * This issue is tracked as https://gitlab.com/qemu-project/qemu/-/issues/926
+ */
+#define GLOBAL_STATE_CODE()
+#else
#define GLOBAL_STATE_CODE() \
do {\
assert(qemu_in_main_thread()); \
} while (0)
+#endif /* CONFIG_COCOA */
/* Mark and check that the function is part of the I/O API. */
#define IO_CODE() \
--
2.35.1
[PULL 3/4] ui/cocoa: Respect left-command-key option
From: Akihiko Odaki
Signed-off-by: Akihiko Odaki
Fixes: 4797adce5f ("ui/cocoa: add option to swap Option and Command")
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20220317152949.68666-1-akihiko.od...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé
---
ui/cocoa.m | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index cb6e7c41dc..c4e5468f9e 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -923,7 +923,8 @@ - (bool) handleEventLocked:(NSEvent *)event
/* Don't pass command key changes to guest unless mouse is
grabbed */
case kVK_Command:
if (isMouseGrabbed &&
-!!(modifiers & NSEventModifierFlagCommand)) {
+!!(modifiers & NSEventModifierFlagCommand) &&
+left_command_key_enabled) {
if (swap_opt_cmd) {
[self toggleKey:Q_KEY_CODE_ALT];
} else {
--
2.35.1
[PULL 1/4] gitattributes: Cover Objective-C source files
From: Philippe Mathieu-Daudé
Apple's Git distribution actually carries a similar file which
annotates *.m:
https://github.com/apple-opensource/Git/blob/73/gitattributes
See comments in commit 29cf16db23 ("buildsys: Help git-diff
adding .gitattributes config file") for details.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Christian Schoenebeck
Message-Id: <20220317130326.39188-1-philippe.mathieu.da...@gmail.com>
---
.gitattributes | 1 +
1 file changed, 1 insertion(+)
diff --git a/.gitattributes b/.gitattributes
index 07f430e944..a217cb7bfe 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,3 +1,4 @@
*.c.inc diff=c
*.h.inc diff=c
+*.m diff=objc
*.pydiff=python
--
2.35.1
Re: [PATCH] vhost-vdpa: fix typo in a comment
On 28/3/22 17:20, Stefano Garzarella wrote: Replace vpda with vdpa. Signed-off-by: Stefano Garzarella --- hw/virtio/vhost-vdpa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 10/15] tests/tcg: remove CONFIG_USER_ONLY from config-target.mak
On 28/3/22 16:02, Paolo Bonzini wrote: Just check the target name instead. Signed-off-by: Paolo Bonzini --- tests/tcg/Makefile.target | 8 tests/tcg/configure.sh| 12 +++- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh index b09956c14d..a17db8ce64 100755 --- a/tests/tcg/configure.sh +++ b/tests/tcg/configure.sh @@ -225,18 +225,12 @@ for target in $target_list; do echo "TARGET_NAME=$arch" >> $config_target_mak echo "target=$target" >> $config_target_mak case $target in -*-linux-user) - echo "CONFIG_USER_ONLY=y" >> $config_target_mak - echo "QEMU=$PWD/qemu-$arch" >> $config_target_mak - ;; -*-bsd-user) - echo "CONFIG_USER_ONLY=y" >> $config_target_mak - echo "QEMU=$PWD/qemu-$arch" >> $config_target_mak - ;; *-softmmu) - echo "CONFIG_SOFTMMU=y" >> $config_target_mak echo "QEMU=$PWD/qemu-system-$arch" >> $config_target_mak ;; +*) Can we restrict to: *-user) just in case? + echo "QEMU=$PWD/qemu-$arch" >> $config_target_mak + ;; esac With '*-user': Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 09/15] tests/tcg: remove CONFIG_LINUX_USER from config-target.mak
On 28/3/22 16:02, Paolo Bonzini wrote: Just check the target name instead. Signed-off-by: Paolo Bonzini --- tests/tcg/configure.sh | 2 -- tests/tcg/multiarch/Makefile.target | 2 +- tests/tcg/x86_64/Makefile.target| 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 07/15] tests/docker: simplify docker-TEST@IMAGE targets
On 28/3/22 16:02, Paolo Bonzini wrote: No need to go through the shell when we already have the test and images at the point where the targets are declared. Signed-off-by: Paolo Bonzini --- tests/docker/Makefile.include | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v5 07/13] KVM: Add KVM_EXIT_MEMORY_ERROR exit
On Thu, Mar 10, 2022, Chao Peng wrote:
> This new KVM exit allows userspace to handle memory-related errors. It
> indicates an error happens in KVM at guest memory range [gpa, gpa+size).
> The flags includes additional information for userspace to handle the
> error. Currently bit 0 is defined as 'private memory' where '1'
> indicates error happens due to private memory access and '0' indicates
> error happens due to shared memory access.
>
> After private memory is enabled, this new exit will be used for KVM to
> exit to userspace for shared memory <-> private memory conversion in
> memory encryption usage.
>
> In such usage, typically there are two kind of memory conversions:
> - explicit conversion: happens when guest explicitly calls into KVM to
> map a range (as private or shared), KVM then exits to userspace to
> do the map/unmap operations.
> - implicit conversion: happens in KVM page fault handler.
> * if the fault is due to a private memory access then causes a
> userspace exit for a shared->private conversion request when the
> page has not been allocated in the private memory backend.
> * If the fault is due to a shared memory access then causes a
> userspace exit for a private->shared conversion request when the
> page has already been allocated in the private memory backend.
>
> Signed-off-by: Yu Zhang
> Signed-off-by: Chao Peng
> ---
> Documentation/virt/kvm/api.rst | 22 ++
> include/uapi/linux/kvm.h | 9 +
> 2 files changed, 31 insertions(+)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index f76ac598606c..bad550c2212b 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6216,6 +6216,28 @@ array field represents return values. The userspace
> should update the return
> values of SBI call before resuming the VCPU. For more details on RISC-V SBI
> spec refer, https://github.com/riscv/riscv-sbi-doc.
>
> +::
> +
> + /* KVM_EXIT_MEMORY_ERROR */
> + struct {
> + #define KVM_MEMORY_EXIT_FLAG_PRIVATE (1 << 0)
> + __u32 flags;
> + __u32 padding;
> + __u64 gpa;
> + __u64 size;
> + } memory;
> +If exit reason is KVM_EXIT_MEMORY_ERROR then it indicates that the VCPU has
Doh, I'm pretty sure I suggested KVM_EXIT_MEMORY_ERROR. Any objection to using
KVM_EXIT_MEMORY_FAULT instead of KVM_EXIT_MEMORY_ERROR? "ERROR" makes me think
of ECC errors, i.e. uncorrected #MC in x86 land, not more generic "faults".
That
would align nicely with -EFAULT.
> +encountered a memory error which is not handled by KVM kernel module and
> +userspace may choose to handle it. The 'flags' field indicates the memory
> +properties of the exit.
Re: [PATCH v2] ui/console: Check console before emitting GL event
On 25/3/22 17:12, Akihiko Odaki wrote: Without this change, The GL output of a console overwrites the other consoles and makes them unusable. Signed-off-by: Akihiko Odaki Reviewed-by: Marc-André Lureau --- ui/console.c | 21 + 1 file changed, 21 insertions(+) Thanks, queued to darwin-fixes.
Re: [PATCH] gitattributes: Cover Objective-C source files
On 17/3/22 14:03, Philippe Mathieu-Daudé wrote:
From: Philippe Mathieu-Daudé
See comments in commit 29cf16db23 ("buildsys: Help git-diff
adding .gitattributes config file") for details.
Signed-off-by: Philippe Mathieu-Daudé
---
.gitattributes | 1 +
1 file changed, 1 insertion(+)
Not a fix bug queued to darwin-fixes.
Re: [PATCH-for-7.0 v3] qemu/main-loop: Disable block backend global state assertion on Cocoa
On 25/3/22 19:37, Philippe Mathieu-Daudé wrote:
From: Philippe Mathieu-Daudé
Since commit 0439c5a462 ("block/block-backend.c: assertions for
block-backend") QEMU crashes when using Cocoa on Darwin hosts.
Example on macOS:
$ qemu-system-i386
Assertion failed: (qemu_in_main_thread()), function blk_all_next, file
block-backend.c, line 552.
Abort trap: 6
Looking with lldb:
Assertion failed: (qemu_in_main_thread()), function blk_all_next, file
block-backend.c, line 552.
Process 76914 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = hit program
assert
frame #4: 0x00010057c2d4 qemu-system-i386`blk_all_next.cold.1
at block-backend.c:552:5 [opt]
549*/
550 BlockBackend *blk_all_next(BlockBackend *blk)
551 {
--> 552 GLOBAL_STATE_CODE();
553 return blk ? QTAILQ_NEXT(blk, link)
554 : QTAILQ_FIRST(_backends);
555 }
Target 1: (qemu-system-i386) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = hit program
assert
frame #0: 0x0001908c99b8 libsystem_kernel.dylib`__pthread_kill + 8
frame #1: 0x0001908fceb0 libsystem_pthread.dylib`pthread_kill + 288
frame #2: 0x00019083a314 libsystem_c.dylib`abort + 164
frame #3: 0x00019083972c libsystem_c.dylib`__assert_rtn + 300
* frame #4: 0x00010057c2d4 qemu-system-i386`blk_all_next.cold.1 at
block-backend.c:552:5 [opt]
frame #5: 0x0001003c00b4
qemu-system-i386`blk_all_next(blk=) at block-backend.c:552:5 [opt]
frame #6: 0x0001003d8f04
qemu-system-i386`qmp_query_block(errp=0x) at qapi.c:591:16 [opt]
frame #7: 0x00010003ab0c qemu-system-i386`main [inlined]
addRemovableDevicesMenuItems at cocoa.m:1756:21 [opt]
frame #8: 0x00010003ab04 qemu-system-i386`main(argc=,
argv=) at cocoa.m:1980:5 [opt]
frame #9: 0x0001012690f4 dyld`start + 520
As we are in passed release 7.0 hard freeze, disable the block
backend assertion which, while being valuable during development,
is not helpful to users. We'll restore this assertion immediately
once 7.0 is released and work on a fix.
Cc: Kevin Wolf
Cc: Paolo Bonzini
Cc: Peter Maydell
Cc: Emanuele Giuseppe Esposito
Suggested-by: Akihiko Odaki
Signed-off-by: Philippe Mathieu-Daudé
---
v3: Reword (Akihiko)
---
include/qemu/main-loop.h | 13 +
1 file changed, 13 insertions(+)
Queued to darwin-fixes.
Re: [PATCH] ui/cocoa: Respect left-command-key option
On 17/3/22 16:29, Akihiko Odaki wrote: Signed-off-by: Akihiko Odaki --- ui/cocoa.m | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Thanks, queued to darwin-fixes.
Re: [PATCH v5 06/13] KVM: Use kvm_userspace_memory_region_ext
On Thu, Mar 10, 2022, Chao Peng wrote:
> @@ -4476,14 +4477,23 @@ static long kvm_vm_ioctl(struct file *filp,
> break;
> }
> case KVM_SET_USER_MEMORY_REGION: {
> - struct kvm_userspace_memory_region kvm_userspace_mem;
> + struct kvm_userspace_memory_region_ext region_ext;
It's probably a good idea to zero initialize the full region to avoid consuming
garbage stack data if there's a bug and an _ext field is accessed without first
checking KVM_MEM_PRIVATE. I'm usually opposed to unnecessary initialization,
but
this seems like something we could screw up quite easily.
> r = -EFAULT;
> - if (copy_from_user(_userspace_mem, argp,
> - sizeof(kvm_userspace_mem)))
> + if (copy_from_user(_ext, argp,
> + sizeof(struct kvm_userspace_memory_region)))
> goto out;
> + if (region_ext.region.flags & KVM_MEM_PRIVATE) {
> + int offset = offsetof(
> + struct kvm_userspace_memory_region_ext,
> + private_offset);
> + if (copy_from_user(_ext.private_offset,
> +argp + offset,
> +sizeof(region_ext) - offset))
In this patch, KVM_MEM_PRIVATE should result in an -EINVAL as it's not yet
supported. Copying the _ext on KVM_MEM_PRIVATE belongs in the "Expose
KVM_MEM_PRIVATE"
patch.
Mechnically, what about first reading flags via get_user(), and then doing a
single
copy_from_user()? It's technically more work in the common case, and requires
an
extra check to guard against TOCTOU attacks, but this isn't a fast path by any
means
and IMO the end result makes it easier to understand the relationship between
KVM_MEM_PRIVATE and the two different structs.
E.g.
case KVM_SET_USER_MEMORY_REGION: {
struct kvm_user_mem_region region;
unsigned long size;
u32 flags;
memset(, 0, sizeof(region));
r = -EFAULT;
if (get_user(flags, (u32 __user *)(argp +
offsetof(typeof(region), flags
goto out;
if (flags & KVM_MEM_PRIVATE)
size = sizeof(struct kvm_userspace_memory_region_ext);
else
size = sizeof(struct kvm_userspace_memory_region);
if (copy_from_user(, argp, size))
goto out;
r = -EINVAL;
if ((flags ^ region.flags) & KVM_MEM_PRIVATE)
goto out;
r = kvm_vm_ioctl_set_memory_region(kvm, );
break;
}
> + goto out;
> + }
>
> - r = kvm_vm_ioctl_set_memory_region(kvm, _userspace_mem);
> + r = kvm_vm_ioctl_set_memory_region(kvm, _ext);
> break;
> }
> case KVM_GET_DIRTY_LOG: {
> --
> 2.17.1
>
Re: [RFC PATCH v7 16/29] hw/loongarch: Add LoongArch ipi interrupt support(IPI)
On 3/28/22 06:57, Xiaojuan Yang wrote:
+typedef struct LoongArchIPI {
+SysBusDevice parent_obj;
+IPICore core[MAX_IPI_CORE_NUM];
+MemoryRegion ipi_mmio[MAX_IPI_CORE_NUM];
+} LoongArchIPI;
Why does this have an array of cores?
Surely the IPI device itself should not have this, but the board model should have an
array of IPI devices, one for each CPU that it supports.
r~
Re: [PATCH] github: fix config mistake preventing repo lockdown commenting
On 23/3/22 12:45, Daniel P. Berrangé wrote: The config key names were all wrong, Since commit 2b678923bb@repo-lockdown (22 Aug 2020, 2 years ago...): feat: move to GitHub Actions BREAKING CHANGE: The deployment method and configuration options have changed. https://github.com/dessant/repo-lockdown/commit/2b678923bbf2df051 resulting in the repo lockdown throwing warnings: Unexpected input(s) 'pull-comment', 'lock-pull', 'close-pull', valid inputs are ['github-token', 'exclude-issue-created-before', 'exclude-issue-labels', 'issue-labels', 'issue-comment', 'skip-closed-issue-comment', 'close-issue', 'lock-issue', 'issue-lock-reason', 'exclude-pr-created-before', 'exclude-pr-labels', 'pr-labels', 'pr-comment', 'skip-closed-pr-comment', 'close-pr', 'lock-pr', 'pr-lock-reason', 'process-only', 'log-output'] It still locked down the pull requests, due to its default config, but didn't leave the friendly message explaining why. Signed-off-by: Daniel P. Berrangé --- .github/workflows/lockdown.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Preferably with description tweaked: Reviewed-by: Philippe Mathieu-Daudé
Re: [RFC PATCH v7 16/29] hw/loongarch: Add LoongArch ipi interrupt support(IPI)
On 3/28/22 06:57, Xiaojuan Yang wrote:
+static void loongarch_ipi_writel(void *opaque, hwaddr addr, uint64_t val,
+ unsigned size)
+{
+IPICore *s = opaque;
+int index = 0;
+
+addr &= 0xff;
+trace_loongarch_ipi_write(size, (uint64_t)addr, val);
+switch (addr) {
+case CORE_STATUS_OFF:
+qemu_log_mask(LOG_GUEST_ERROR, "can not be written");
+break;
+case CORE_EN_OFF:
+s->en = val;
+break;
Changes to s->en should affect irq.
+case CORE_SET_OFF:
+s->status |= val;
+if (s->status != 0) {
+qemu_irq_raise(s->irq);
+}
I think s->en should be taken into account when raising irq.
+break;
+case CORE_CLEAR_OFF:
+s->status ^= val;
Incorrect: status &= ~val.
+if (s->status == 0) {
+qemu_irq_lower(s->irq);
+}
Likewise, s->en.
+break;
+case CORE_BUF_20 ... CORE_BUF_38 + 4:
+index = (addr - CORE_BUF_20) >> 2;
+s->buf[index] = val;
+break;
+case IOCSR_IPI_SEND:
+s->status |= val;
I can't see where this comes from, but helper_iocsr_write is very confusing. It *appears*
as if this is never invoked, because IPI_SEND is handled directly in helper_iocsr_write
(which also seems wrong).
r~
Re: [PATCH v2] ui/console: Check console before emitting GL event
On 25/3/22 17:12, Akihiko Odaki wrote: Without this change, The GL output of a console overwrites the other consoles and makes them unusable. Signed-off-by: Akihiko Odaki Reviewed-by: Marc-André Lureau --- ui/console.c | 21 + 1 file changed, 21 insertions(+) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] tests/tcg/xtensa: allow testing big-endian cores
On 25/3/22 19:23, Max Filippov wrote: Don't disable all big-endian tests, instead check whether $(CORE) is supported by the configured $(QEMU) and enable tests if it is. Signed-off-by: Max Filippov --- MAINTAINERS| 1 + tests/tcg/xtensa/Makefile.softmmu-target | 4 ++-- tests/tcg/xtensaeb/Makefile.softmmu-target | 5 + 3 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 tests/tcg/xtensaeb/Makefile.softmmu-target Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v5 05/13] KVM: Extend the memslot to support fd-based private memory
On Thu, Mar 10, 2022, Chao Peng wrote:
> Extend the memslot definition to provide fd-based private memory support
> by adding two new fields (private_fd/private_offset). The memslot then
> can maintain memory for both shared pages and private pages in a single
> memslot. Shared pages are provided by existing userspace_addr(hva) field
> and private pages are provided through the new private_fd/private_offset
> fields.
>
> Since there is no 'hva' concept anymore for private memory so we cannot
> rely on get_user_pages() to get a pfn, instead we use the newly added
> memfile_notifier to complete the same job.
>
> This new extension is indicated by a new flag KVM_MEM_PRIVATE.
>
> Signed-off-by: Yu Zhang
> Signed-off-by: Chao Peng
> ---
> Documentation/virt/kvm/api.rst | 37 +++---
> include/linux/kvm_host.h | 7 +++
> include/uapi/linux/kvm.h | 8
> 3 files changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 3acbf4d263a5..f76ac598606c 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -1307,7 +1307,7 @@ yet and must be cleared on entry.
> :Capability: KVM_CAP_USER_MEMORY
> :Architectures: all
> :Type: vm ioctl
> -:Parameters: struct kvm_userspace_memory_region (in)
> +:Parameters: struct kvm_userspace_memory_region(_ext) (in)
> :Returns: 0 on success, -1 on error
>
> ::
> @@ -1320,9 +1320,17 @@ yet and must be cleared on entry.
> __u64 userspace_addr; /* start of the userspace allocated memory */
>};
>
> + struct kvm_userspace_memory_region_ext {
> + struct kvm_userspace_memory_region region;
Peeking ahead, the partial switch to the _ext variant is rather gross. I would
prefer that KVM use an entirely different, but binary compatible, struct
internally.
And once the kernel supports C11[*], I'm pretty sure we can make the "region" in
_ext an anonymous struct, and make KVM's internal struct a #define of _ext.
That
should minimize the churn (no need to get the embedded "region" field), reduce
line lengths, and avoid confusion due to some flows taking the _ext but others
dealing with only the "base" struct.
Maybe kvm_user_memory_region or kvm_user_mem_region? Though it's tempting to be
evil and usurp the old kvm_memory_region :-)
E.g. pre-C11 do
struct kvm_userspace_memory_region_ext {
struct kvm_userspace_memory_region region;
__u64 private_offset;
__u32 private_fd;
__u32 padding[5];
};
#ifdef __KERNEL__
struct kvm_user_mem_region {
__u32 slot;
__u32 flags;
__u64 guest_phys_addr;
__u64 memory_size; /* bytes */
__u64 userspace_addr; /* start of the userspace allocated memory */
__u64 private_offset;
__u32 private_fd;
__u32 padding[5];
};
#endif
and then post-C11 do
struct kvm_userspace_memory_region_ext {
#ifdef __KERNEL__
struct kvm_userspace_memory_region region;
#else
struct kvm_userspace_memory_region;
#endif
__u64 private_offset;
__u32 private_fd;
__u32 padding[5];
};
#ifdef __KERNEL__
#define kvm_user_mem_region kvm_userspace_memory_region_ext
#endif
[*] https://lore.kernel.org/all/20220301145233.3689119-1-a...@kernel.org
> + __u64 private_offset;
> + __u32 private_fd;
> + __u32 padding[5];
> +};
Re: [PATCH] target/ppc: Improve KVM hypercall trace
On 3/25/22 19:33, Fabiano Rosas wrote: Before: kvm_handle_papr_hcall handle PAPR hypercall kvm_handle_papr_hcall handle PAPR hypercall kvm_handle_papr_hcall handle PAPR hypercall kvm_handle_papr_hcall handle PAPR hypercall kvm_handle_papr_hcall handle PAPR hypercall kvm_handle_papr_hcall handle PAPR hypercall After: kvm_handle_papr_hcall 0x3a8 kvm_handle_papr_hcall 0x3ac kvm_handle_papr_hcall 0x108 kvm_handle_papr_hcall 0x104 kvm_handle_papr_hcall 0x104 kvm_handle_papr_hcall 0x108 Signed-off-by: Fabiano Rosas --- Reviewed-by: Daniel Henrique Barboza I believe this is benign enough to be taken in the next PR and it will help in the debugging right away. Daniel target/ppc/kvm.c| 2 +- target/ppc/trace-events | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index dc93b99189..a490e886ea 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -1681,7 +1681,7 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) break; #if defined(TARGET_PPC64) case KVM_EXIT_PAPR_HCALL: -trace_kvm_handle_papr_hcall(); +trace_kvm_handle_papr_hcall(run->papr_hcall.nr); run->papr_hcall.ret = spapr_hypercall(cpu, run->papr_hcall.nr, run->papr_hcall.args); diff --git a/target/ppc/trace-events b/target/ppc/trace-events index 53b107f56e..a79f1b4370 100644 --- a/target/ppc/trace-events +++ b/target/ppc/trace-events @@ -23,7 +23,7 @@ kvm_failed_get_vpa(void) "Warning: Unable to get VPA information from KVM" kvm_handle_dcr_write(void) "handle dcr write" kvm_handle_dcr_read(void) "handle dcr read" kvm_handle_halt(void) "handle halt" -kvm_handle_papr_hcall(void) "handle PAPR hypercall" +kvm_handle_papr_hcall(uint64_t hcall) "0x%" PRIx64 kvm_handle_epr(void) "handle epr" kvm_handle_watchdog_expiry(void) "handle watchdog expiry" kvm_handle_debug_exception(void) "handle debug exception"
Re: [PATCH v5 05/13] KVM: Extend the memslot to support fd-based private memory
On Thu, Mar 10, 2022, Chao Peng wrote:
> Extend the memslot definition to provide fd-based private memory support
> by adding two new fields (private_fd/private_offset). The memslot then
> can maintain memory for both shared pages and private pages in a single
> memslot. Shared pages are provided by existing userspace_addr(hva) field
> and private pages are provided through the new private_fd/private_offset
> fields.
>
> Since there is no 'hva' concept anymore for private memory so we cannot
> rely on get_user_pages() to get a pfn, instead we use the newly added
> memfile_notifier to complete the same job.
>
> This new extension is indicated by a new flag KVM_MEM_PRIVATE.
>
> Signed-off-by: Yu Zhang
Needs a Co-developed-by: for Yu, or a From: if Yu is the sole author.
> Signed-off-by: Chao Peng
> ---
> Documentation/virt/kvm/api.rst | 37 +++---
> include/linux/kvm_host.h | 7 +++
> include/uapi/linux/kvm.h | 8
> 3 files changed, 45 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 3acbf4d263a5..f76ac598606c 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -1307,7 +1307,7 @@ yet and must be cleared on entry.
> :Capability: KVM_CAP_USER_MEMORY
> :Architectures: all
> :Type: vm ioctl
> -:Parameters: struct kvm_userspace_memory_region (in)
> +:Parameters: struct kvm_userspace_memory_region(_ext) (in)
> :Returns: 0 on success, -1 on error
>
> ::
> @@ -1320,9 +1320,17 @@ yet and must be cleared on entry.
> __u64 userspace_addr; /* start of the userspace allocated memory */
>};
>
> + struct kvm_userspace_memory_region_ext {
> + struct kvm_userspace_memory_region region;
> + __u64 private_offset;
> + __u32 private_fd;
> + __u32 padding[5];
Uber nit, I'd prefer we pad u32 for private_fd separate from padding the size of
the structure for future expansion.
Regarding future expansion, any reason not to go crazy and pad like 128+ bytes?
It'd be rather embarassing if the next memslot extension needs 3 u64s and we end
up with region_ext2 :-)
> +};
> +
>/* for kvm_memory_region::flags */
>#define KVM_MEM_LOG_DIRTY_PAGES(1UL << 0)
>#define KVM_MEM_READONLY (1UL << 1)
> + #define KVM_MEM_PRIVATE(1UL << 2)
>
> This ioctl allows the user to create, modify or delete a guest physical
> memory slot. Bits 0-15 of "slot" specify the slot id and this value
...
> +static inline bool kvm_slot_is_private(const struct kvm_memory_slot *slot)
I 100% think we should usurp the name "private" for these memslots, but as prep
work this series should first rename KVM_PRIVATE_MEM_SLOTS to avoid confusion.
Maybe KVM_INTERNAL_MEM_SLOTS?
[PATCH v1 8/9] qom: add command to print initial properties
The command "query-init-properties" is needed to get values of properties
after initialization (not only default value). It makes sense, for example,
when working with x86_64-cpu.
All machine types (and x-remote-object, because its init uses machime
type's infrastructure) should be skipped, because only the one instance can
be correctly initialized.
Signed-off-by: Maxim Davydov
---
qapi/qom.json | 69 ++
qom/qom-qmp-cmds.c | 121 +
2 files changed, 190 insertions(+)
diff --git a/qapi/qom.json b/qapi/qom.json
index eeb5395ff3..1eedc441eb 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -949,3 +949,72 @@
##
{ 'command': 'object-del', 'data': {'id': 'str'},
'allow-preconfig': true }
+
+##
+# @InitValue:
+#
+# Not all objects have default values but they have "initial" values.
+#
+# @name: property name
+#
+# @value: Current value (default or after initialization. It makes sence,
+# for example, for x86-cpus)
+#
+# Since: 7.0
+#
+##
+{ 'struct': 'InitValue',
+ 'data': { 'name': 'str',
+'*value': 'any' } }
+
+##
+# @ClassProperties:
+#
+# Initial values of properties that are owned by the class
+#
+# @classname: name of the class that owns appropriate properties
+#
+# @classprops: List of class properties
+#
+# Since: 7.0
+#
+##
+{ 'struct': 'ClassProperties',
+ 'data': { 'classname': 'str',
+'*classprops': [ 'InitValue' ] } }
+
+##
+# @InitProps:
+#
+# List of properties and their values that are available after class
+# initialization. So it important to know default value of the property
+# even if it doesn't have "QObject *defval"
+#
+# @name: Object name
+#
+# @props: List of properties
+#
+# Notes: a value in each property was defval if it's available
+#otherwise it's obtained via "(ObjectPropertyAccessor*) get"
+#immediately after initialization of device object.
+#
+# Since: 7.0
+#
+##
+{ 'struct': 'InitProps',
+ 'data': { 'name': 'str',
+'props': [ 'ClassProperties' ] } }
+
+##
+# @query-init-properties:
+#
+# Returns list of all objects (except all types related with machine type)
+# with all properties and their "default" values that will be available
+# after initialization. The main purpose of this command is to be used to
+# build table with all machine-type-specific properties
+#
+# Since: 7.0
+#
+##
+{ 'command': 'query-init-properties',
+ 'returns': [ 'InitProps' ] }
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 2d6f41ecc7..c1bb3f1f8b 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -27,6 +27,7 @@
#include "qemu/cutils.h"
#include "qom/object_interfaces.h"
#include "qom/qom-qobject.h"
+#include "hw/boards.h"
ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)
{
@@ -235,3 +236,123 @@ void qmp_object_del(const char *id, Error **errp)
{
user_creatable_del(id, errp);
}
+
+static void query_object_prop(InitValueList **props_list, ObjectProperty *prop,
+ Object *obj, Error **errp)
+{
+InitValue *prop_info = NULL;
+
+/* Skip inconsiderable properties */
+if (strcmp(prop->name, "type") == 0 ||
+strcmp(prop->name, "realized") == 0 ||
+strcmp(prop->name, "hotpluggable") == 0 ||
+strcmp(prop->name, "hotplugged") == 0 ||
+strcmp(prop->name, "parent_bus") == 0) {
+return;
+}
+
+prop_info = g_malloc0(sizeof(*prop_info));
+prop_info->name = g_strdup(prop->name);
+prop_info->value = NULL;
+if (prop->defval) {
+prop_info->value = qobject_ref(prop->defval);
+} else if (prop->get) {
+/*
+ * crash-information in x86-cpu uses errp to return current state.
+ * So, after requesting this property it returns GenericError:
+ * "No crash occured"
+ */
+if (strcmp(prop->name, "crash-information") != 0) {
+prop_info->value = object_property_get_qobject(obj, prop->name,
+ errp);
+}
+}
+prop_info->has_value = !!prop_info->value;
+
+QAPI_LIST_PREPEND(*props_list, prop_info);
+}
+
+typedef struct QIPData {
+InitPropsList **dev_list;
+Error **errp;
+} QIPData;
+
+static void query_init_properties_tramp(gpointer list_data, gpointer opaque)
+{
+ObjectClass *k = list_data;
+Object *obj;
+ObjectClass *parent;
+GHashTableIter iter;
+
+QIPData *data = opaque;
+ClassPropertiesList *class_props_list = NULL;
+InitProps *dev_info;
+
+/* Only one machine can be initialized correctly (it's already happened) */
+if (object_class_dynamic_cast(k, TYPE_MACHINE)) {
+return;
+}
+
+const char *klass_name = object_class_get_name(k);
+/*
+ * Uses machine type infrastructure with notifiers. It causes immediate
+ * notify and SEGSEGV during remote_object_machine_done
+ */
+if (strcmp(klass_name, "x-remote-object") == 0) {
+
[PATCH v1 9/9] scripts: printing machine type compat properties
This script makes the information that can be obtained from
query-init-properties and query-machines easy to read.
Note: some init values from the devices can't be available like properties
from virtio-9p when configure has --disable-virtfs. Such values are
replaced by "DEFAULT". Another exception is properties of abstract
classes. The default value of the abstract class property is common
value of all child classes. But if the values of the child classes are
different the default value will be "BASED_ON_CHILD" (for example, old
x86_64-cpu can have unsupported feature).
Example:
1) virsh qemu-monitor-command VM --pretty \
'{"execute" : "query-init-properties"}' > init_props.json
2) virsh qemu-monitor-command VM --pretty \
'{"execute" : "query-machines", "arguments" : {"is-full" : true}}' \
> compat_props.json
3) scripts/print_MT.py --MT_compat_props compat_props.json\
--init_props init_props.json --mt pc-q35-7.0 pc-q35-6.1
Output:
╒═══╤══╤══╕
│ property_name │ pc-q35-7.0 │ pc-q35-6.1 │
╞═══╪══╪══╡
│ ICH9-LPC-x-keep-pci-slot-hpc│ True │False │
├───┼──┼──┤
│ nvme-ns-shared │ True │ off │
├───┼──┼──┤
│ vhost-user-vsock-device-seqpacket │ auto │ off │
├───┼──┼──┤
│ virtio-mem-unplugged-inaccessible │ auto │ off │
├───┼──┼──┤
│ x86_64-cpu-hv-version-id-build │14393 │0x1bbc│
├───┼──┼──┤
│ x86_64-cpu-hv-version-id-major │ 10 │0x0006│
├───┼──┼──┤
│ x86_64-cpu-hv-version-id-minor │ 0 │0x0001│
╘═══╧══╧══╛
Signed-off-by: Maxim Davydov
---
scripts/print_MT.py | 274
1 file changed, 274 insertions(+)
create mode 100755 scripts/print_MT.py
diff --git a/scripts/print_MT.py b/scripts/print_MT.py
new file mode 100755
index 00..8be13be8d7
--- /dev/null
+++ b/scripts/print_MT.py
@@ -0,0 +1,274 @@
+#! /usr/bin/python3
+#
+# Script for printing machine type compatible features. It uses two JSON files
+# that should be generated by qmp-init-properties and query-machines.
+#
+# Copyright (c) 2022 Maxim Davydov
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see
[PATCH v1 7/9] colo-compare: safe finalization
Fixes some possible issues with finalization. For example, finalization
immediately after instance_init fails on the assert.
Signed-off-by: Maxim Davydov
---
net/colo-compare.c | 25 -
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 62554b5b3c..81d8de0aaa 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -1426,7 +1426,7 @@ static void colo_compare_finalize(Object *obj)
break;
}
}
-if (QTAILQ_EMPTY(_compares)) {
+if (QTAILQ_EMPTY(_compares) && colo_compare_active) {
colo_compare_active = false;
qemu_mutex_destroy(_mtx);
qemu_cond_destroy(_complete_cond);
@@ -1442,19 +1442,26 @@ static void colo_compare_finalize(Object *obj)
colo_compare_timer_del(s);
-qemu_bh_delete(s->event_bh);
+if (s->event_bh) {
+qemu_bh_delete(s->event_bh);
+}
-AioContext *ctx = iothread_get_aio_context(s->iothread);
-aio_context_acquire(ctx);
-AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
-if (s->notify_dev) {
-AIO_WAIT_WHILE(ctx, !s->notify_sendco.done);
+if (s->iothread) {
+AioContext *ctx = iothread_get_aio_context(s->iothread);
+aio_context_acquire(ctx);
+AIO_WAIT_WHILE(ctx, !s->out_sendco.done);
+if (s->notify_dev) {
+AIO_WAIT_WHILE(ctx, !s->notify_sendco.done);
+}
+aio_context_release(ctx);
}
-aio_context_release(ctx);
/* Release all unhandled packets after compare thead exited */
g_queue_foreach(>conn_list, colo_flush_packets, s);
-AIO_WAIT_WHILE(NULL, !s->out_sendco.done);
+/* Without colo_compare_complete done == false without packets */
+if (!g_queue_is_empty(>out_sendco.send_list)) {
+AIO_WAIT_WHILE(NULL, !s->out_sendco.done);
+}
g_queue_clear(>conn_list);
g_queue_clear(>out_sendco.send_list);
--
2.31.1
[PATCH v1 4/9] msmouse: add appropriate unregister handler
Attempt to finalize msmouse after initalization brings to segmentation
fault in QTAILQ_REMOVE.
Signed-off-by: Maxim Davydov
---
chardev/msmouse.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/chardev/msmouse.c b/chardev/msmouse.c
index eb9231dcdb..2cc1b16561 100644
--- a/chardev/msmouse.c
+++ b/chardev/msmouse.c
@@ -146,7 +146,9 @@ static void char_msmouse_finalize(Object *obj)
{
MouseChardev *mouse = MOUSE_CHARDEV(obj);
-qemu_input_handler_unregister(mouse->hs);
+if (mouse->hs) {
+qemu_input_handler_unregister(mouse->hs);
+}
}
static QemuInputHandler msmouse_handler = {
--
2.31.1
[PATCH v1 5/9] wctablet: add appropriate unregister handler
Attempt to finalize msmouse after initalization brings to segmentation
fault in QTAILQ_REMOVE.
Signed-off-by: Maxim Davydov
---
chardev/wctablet.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/chardev/wctablet.c b/chardev/wctablet.c
index e8b292c43c..43bdf6b608 100644
--- a/chardev/wctablet.c
+++ b/chardev/wctablet.c
@@ -319,7 +319,9 @@ static void wctablet_chr_finalize(Object *obj)
{
TabletChardev *tablet = WCTABLET_CHARDEV(obj);
-qemu_input_handler_unregister(tablet->hs);
+if (tablet->hs) {
+qemu_input_handler_unregister(tablet->hs);
+}
}
static void wctablet_chr_open(Chardev *chr,
--
2.31.1
[PATCH v1 6/9] chardev: add appropriate getting address
Attempt to get address after initialization shouldn't fail on assert in
the qapi automatically generated code. As a possible solution, it can
return null type.
Signed-off-by: Maxim Davydov
---
chardev/char-socket.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index fab2d791d4..f851e3346b 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -33,6 +33,7 @@
#include "qapi/clone-visitor.h"
#include "qapi/qapi-visit-sockets.h"
#include "qemu/yank.h"
+#include "qapi/qmp/qnull.h"
#include "chardev/char-io.h"
#include "chardev/char-socket.h"
@@ -1509,6 +1510,14 @@ char_socket_get_addr(Object *obj, Visitor *v, const char
*name,
{
SocketChardev *s = SOCKET_CHARDEV(obj);
+QNull *null = NULL;
+
+/* Return NULL type if getting addr was called after init */
+if (!s->addr) {
+visit_type_null(v, NULL, , errp);
+return;
+}
+
visit_type_SocketAddress(v, name, >addr, errp);
}
--
2.31.1
[PATCH v1 1/9] qmp: Add dump machine type compatible properties
This patch adds the ability to get all the compat_props of the
corresponding supported machines for their comparison.
Example:
{ "execute" : "query-machines", "arguments" : { "is-full" : true } }
Signed-off-by: Maxim Davydov
---
hw/core/machine-qmp-cmds.c | 25 +++-
qapi/machine.json | 58 +++--
tests/qtest/fuzz/qos_fuzz.c | 2 +-
3 files changed, 81 insertions(+), 4 deletions(-)
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 4f4ab30f8c..8f3206ba8d 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -74,7 +74,8 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
return head;
}
-MachineInfoList *qmp_query_machines(Error **errp)
+MachineInfoList *qmp_query_machines(bool has_is_full, bool is_full,
+Error **errp)
{
GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
MachineInfoList *mach_list = NULL;
@@ -107,6 +108,28 @@ MachineInfoList *qmp_query_machines(Error **errp)
info->default_ram_id = g_strdup(mc->default_ram_id);
info->has_default_ram_id = true;
}
+if (has_is_full && is_full && mc->compat_props) {
+int i;
+info->compat_props = NULL;
+info->has_compat_props = true;
+
+for (i = 0; i < mc->compat_props->len; i++) {
+GlobalProperty *mt_prop = g_ptr_array_index(mc->compat_props,
+i);
+ObjectClass *klass = object_class_by_name(mt_prop->driver);
+CompatProperty *prop;
+
+prop = g_malloc0(sizeof(*prop));
+if (klass && object_class_is_abstract(klass)) {
+prop->abstract = true;
+}
+prop->driver = g_strdup(mt_prop->driver);
+prop->property = g_strdup(mt_prop->property);
+prop->value = g_strdup(mt_prop->value);
+
+QAPI_LIST_PREPEND(info->compat_props, prop);
+}
+}
QAPI_LIST_PREPEND(mach_list, info);
}
diff --git a/qapi/machine.json b/qapi/machine.json
index 42fc68403d..16e961477c 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -130,6 +130,28 @@
##
{ 'command': 'query-cpus-fast', 'returns': [ 'CpuInfoFast' ] }
+##
+# @CompatProperty:
+#
+# Machine type compatible property. It's based on GlobalProperty and created
+# for machine type compat properties (see scripts)
+#
+# @driver: name of the driver that has GlobalProperty
+#
+# @abstract: Bool value that shows that property is belonged to abstract class
+#
+# @property: global property name
+#
+# @value: global property value
+#
+# Since: 7.0
+##
+{ 'struct': 'CompatProperty',
+ 'data': { 'driver': 'str',
+'abstract': 'bool',
+'property': 'str',
+'value': 'str' } }
+
##
# @MachineInfo:
#
@@ -158,6 +180,9 @@
#
# @default-ram-id: the default ID of initial RAM memory backend (since 5.2)
#
+# @compat-props: List of compatible properties that defines machine type
+#(since 7.0)
+#
# Since: 1.2
##
{ 'struct': 'MachineInfo',
@@ -165,18 +190,47 @@
'*is-default': 'bool', 'cpu-max': 'int',
'hotpluggable-cpus': 'bool', 'numa-mem-supported': 'bool',
'deprecated': 'bool', '*default-cpu-type': 'str',
-'*default-ram-id': 'str' } }
+'*default-ram-id': 'str', '*compat-props': ['CompatProperty'] } }
##
# @query-machines:
#
# Return a list of supported machines
#
+# @is-full: if true return will contain information about machine type
+# compatible properties (since 7.0)
+#
# Returns: a list of MachineInfo
#
# Since: 1.2
+#
+# Example:
+#
+# -> { "execute" : "query-machines", "arguments" : { "is-full" : true } }
+# <- { "return": [
+# {
+# "hotpluggable-cpus": true,
+# "name": "pc-q35-6.2",
+# "compat-props": [
+# {
+# "abstract": false,
+# "driver": "virtio-mem",
+# "property": "unplugged-inaccessible",
+# "value": "off"
+# }
+# ],
+# "numa-mem-supported": false,
+# "default-cpu-type": "qemu64-x86_64-cpu",
+# "cpu-max": 288,
+# "deprecated": false,
+# "default-ram-id": "pc.ram"
+# },
+# ...
+#}
##
-{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
+{ 'command': 'query-machines',
+ 'data': { '*is-full': 'bool' },
+ 'returns': ['MachineInfo'] }
##
# @CurrentMachineParams:
diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index 7a244c951e..3f9c1ede6e 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -47,7 +47,7 @@ static void
[PATCH v1 2/9] pci: add null-pointer check
Call pci_bus_get_w64_range can fail with the segmentation fault. For
example, this can happen during attempt to get pci-hole64-end immediately
after initialization.
Signed-off-by: Maxim Davydov
---
hw/pci-host/i440fx.c | 17 +++--
hw/pci-host/q35.c| 17 +++--
2 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index e08716142b..71a114e551 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -158,10 +158,12 @@ static uint64_t
i440fx_pcihost_get_pci_hole64_start_value(Object *obj)
PCIHostState *h = PCI_HOST_BRIDGE(obj);
I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
Range w64;
-uint64_t value;
+uint64_t value = 0;
-pci_bus_get_w64_range(h->bus, );
-value = range_is_empty() ? 0 : range_lob();
+if (h->bus) {
+pci_bus_get_w64_range(h->bus, );
+value = range_is_empty() ? 0 : range_lob();
+}
if (!value && s->pci_hole64_fix) {
value = pc_pci_hole64_start();
}
@@ -191,10 +193,13 @@ static void i440fx_pcihost_get_pci_hole64_end(Object
*obj, Visitor *v,
I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
uint64_t hole64_start = i440fx_pcihost_get_pci_hole64_start_value(obj);
Range w64;
-uint64_t value, hole64_end;
+uint64_t value = 0;
+uint64_t hole64_end;
-pci_bus_get_w64_range(h->bus, );
-value = range_is_empty() ? 0 : range_upb() + 1;
+if (h->bus) {
+pci_bus_get_w64_range(h->bus, );
+value = range_is_empty() ? 0 : range_upb() + 1;
+}
hole64_end = ROUND_UP(hole64_start + s->pci_hole64_size, 1ULL << 30);
if (s->pci_hole64_fix && value < hole64_end) {
value = hole64_end;
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index ab5a47aff5..d679fd85ef 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -124,10 +124,12 @@ static uint64_t
q35_host_get_pci_hole64_start_value(Object *obj)
PCIHostState *h = PCI_HOST_BRIDGE(obj);
Q35PCIHost *s = Q35_HOST_DEVICE(obj);
Range w64;
-uint64_t value;
+uint64_t value = 0;
-pci_bus_get_w64_range(h->bus, );
-value = range_is_empty() ? 0 : range_lob();
+if (h->bus) {
+pci_bus_get_w64_range(h->bus, );
+value = range_is_empty() ? 0 : range_lob();
+}
if (!value && s->pci_hole64_fix) {
value = pc_pci_hole64_start();
}
@@ -157,10 +159,13 @@ static void q35_host_get_pci_hole64_end(Object *obj,
Visitor *v,
Q35PCIHost *s = Q35_HOST_DEVICE(obj);
uint64_t hole64_start = q35_host_get_pci_hole64_start_value(obj);
Range w64;
-uint64_t value, hole64_end;
+uint64_t value = 0;
+uint64_t hole64_end;
-pci_bus_get_w64_range(h->bus, );
-value = range_is_empty() ? 0 : range_upb() + 1;
+if (h->bus) {
+pci_bus_get_w64_range(h->bus, );
+value = range_is_empty() ? 0 : range_upb() + 1;
+}
hole64_end = ROUND_UP(hole64_start + s->mch.pci_hole64_size, 1ULL << 30);
if (s->pci_hole64_fix && value < hole64_end) {
value = hole64_end;
--
2.31.1
[PATCH v1 3/9] mem: appropriate handling getting mem region
Attempt to get memory region if the device doesn't have hostmem may not be
an error. This can be happen immediately after initialization (getting
value without default one).
Signed-off-by: Maxim Davydov
---
hw/i386/sgx-epc.c | 5 -
hw/mem/nvdimm.c | 6 ++
hw/mem/pc-dimm.c | 5 +
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/hw/i386/sgx-epc.c b/hw/i386/sgx-epc.c
index d664829d35..1a4c8acdcc 100644
--- a/hw/i386/sgx-epc.c
+++ b/hw/i386/sgx-epc.c
@@ -121,9 +121,12 @@ static MemoryRegion
*sgx_epc_md_get_memory_region(MemoryDeviceState *md,
{
SGXEPCDevice *epc = SGX_EPC(md);
HostMemoryBackend *hostmem;
+DeviceState *dev = DEVICE(epc);
if (!epc->hostmem) {
-error_setg(errp, "'" SGX_EPC_MEMDEV_PROP "' property must be set");
+if (dev->realized) {
+error_setg(errp, "'" SGX_EPC_MEMDEV_PROP "' property must be set");
+}
return NULL;
}
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 7c7d81..61e77e5476 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -166,9 +166,15 @@ static MemoryRegion
*nvdimm_md_get_memory_region(MemoryDeviceState *md,
Error **errp)
{
NVDIMMDevice *nvdimm = NVDIMM(md);
+PCDIMMDevice *dimm = PC_DIMM(nvdimm);
Error *local_err = NULL;
if (!nvdimm->nvdimm_mr) {
+/* Not error if we try get memory region after init */
+if (!dimm->hostmem) {
+return NULL;
+}
+
nvdimm_prepare_memory_region(nvdimm, _err);
if (local_err) {
error_propagate(errp, local_err);
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index f27e1a11ba..6fd74de97f 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -240,6 +240,11 @@ static void pc_dimm_md_set_addr(MemoryDeviceState *md,
uint64_t addr,
static MemoryRegion *pc_dimm_md_get_memory_region(MemoryDeviceState *md,
Error **errp)
{
+PCDIMMDevice *dimm = PC_DIMM(md);
+/* Not error if we try get memory region after init */
+if (!dimm->hostmem) {
+return NULL;
+}
return pc_dimm_get_memory_region(PC_DIMM(md), errp);
}
--
2.31.1
[PATCH v1 0/9] Machine type compatible properties
We need to be able to check machine type after its definition. It's necessary when using complicated inheritance of compatible features. For instance, this tool can help to find bugs in the machine type definition if the name of the device has been changed. Also, this tool was created to help with MTs of other projects such as vz branches. Maxim Davydov (9): qmp: Add dump machine type compatible properties pci: add null-pointer check mem: appropriate handling getting mem region msmouse: add appropriate unregister handler wctablet: add appropriate unregister handler chardev: add appropriate getting address colo-compare: safe finalization qom: add command to print initial properties scripts: printing machine type compat properties chardev/char-socket.c | 9 ++ chardev/msmouse.c | 4 +- chardev/wctablet.c | 4 +- hw/core/machine-qmp-cmds.c | 25 +++- hw/i386/sgx-epc.c | 5 +- hw/mem/nvdimm.c | 6 + hw/mem/pc-dimm.c| 5 + hw/pci-host/i440fx.c| 17 ++- hw/pci-host/q35.c | 17 ++- net/colo-compare.c | 25 ++-- qapi/machine.json | 58 +++- qapi/qom.json | 69 + qom/qom-qmp-cmds.c | 121 scripts/print_MT.py | 274 tests/qtest/fuzz/qos_fuzz.c | 2 +- 15 files changed, 613 insertions(+), 28 deletions(-) create mode 100755 scripts/print_MT.py -- 2.31.1
Re: [RFC PATCH v7 14/29] hw/loongarch: Add support loongson3 virt machine type.
On 28/03/2022 21:49, Richard Henderson wrote:
On 3/28/22 06:57, Xiaojuan Yang wrote:
+static uint64_t loongarch_qemu_read(void *opaque, hwaddr addr, unsigned size)
+{
+ uint64_t feature = 0UL;
+
+ switch (addr) {
+ case FEATURE_REG:
+ feature |= 1UL << IOCSRF_MSI | 1UL << IOCSRF_EXTIOI |
+ 1UL << IOCSRF_CSRIPI;
+ return feature ;
What's the point of the feature variable?
+ case VENDOR_REG:
+ return *(uint64_t *)"Loongson";
+ case CPUNAME_REG:
+ return *(uint64_t *)"3A5000";
This is definitely wrong, as (1) it depends on host endianness, and (2) you're
reading 8 bytes from a 7 byte string.
+static const MemoryRegionOps loongarch_qemu_ops = {
+ .read = loongarch_qemu_read,
+ .write = loongarch_qemu_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid = {
+ .min_access_size = 4,
+ .max_access_size = 8,
+ },
+ .impl = {
+ .min_access_size = 4,
+ .max_access_size = 8,
+ },
The implementation above doesn't actually support access size 4; it only
supports 8.
It doesn't seem like this should be a io region at all, but a ROM.
Strangely enough I had a similar requirement for my q800 patches, and when I tried to
implement a ROM memory region then the accesses didn't work as expected. I can't
remember the exact problem however...
+static void loongarch_cpu_init(LoongArchCPU *la_cpu, int cpu_num)
+{
+ CPULoongArchState *env;
+ env = _cpu->env;
+
+ memory_region_init_io(>system_iocsr, OBJECT(la_cpu), NULL,
+ env, "iocsr", UINT64_MAX);
+ address_space_init(>address_space_iocsr, >system_iocsr, "IOCSR");
+
+ timer_init_ns(_cpu->timer, QEMU_CLOCK_VIRTUAL,
+ _constant_timer_cb, la_cpu);
This timer belongs to the cpu, not the board model.
This init belongs over in target/loongarch/.
That's probably my fault; the example of splitting the non-user parts of the CPU into
a separate function was based upon SPARC64 and that code currently lives in
hw/sparc64. I do recall there were some recent discussions about moving such code
into target/* though.
ATB,
Mark.
Re: [RFC PATCH v7 05/29] target/loongarch: Add constant timer support
On 3/28/22 06:57, Xiaojuan Yang wrote: +#define N_IRQS 14 There are only 13 irqs, according to ESTAT. r~
Re: [RFC PATCH v7 15/29] hw/loongarch: Add LoongArch cpu interrupt support(CPUINTC)
On 3/28/22 06:57, Xiaojuan Yang wrote:
+static void loongarch_cpu_set_irq(void *opaque, int irq, int level)
+{
+LoongArchCPU *cpu = opaque;
+CPULoongArchState *env = >env;
+CPUState *cs = CPU(cpu);
+
+if (irq < 0 || irq > N_IRQS) {
+return;
+}
+
+if (level) {
+env->CSR_ESTAT |= 1 << irq;
+} else {
+env->CSR_ESTAT &= ~(1 << irq);
+}
+
+if (FIELD_EX64(env->CSR_ESTAT, CSR_ESTAT, IS)) {
+cpu_interrupt(cs, CPU_INTERRUPT_HARD);
+} else {
+cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
+}
+}
+
static void loongarch_cpu_reset(void *opaque)
{
LoongArchCPU *cpu = opaque;
@@ -69,6 +92,8 @@ static void loongarch_cpu_init(LoongArchCPU *la_cpu, int
cpu_num)
CPULoongArchState *env;
env = _cpu->env;
+qdev_init_gpio_in(DEVICE(la_cpu), loongarch_cpu_set_irq, N_IRQS);
This all belongs over in target/loongarch/.
r~
Re: [RFC PATCH v7 14/29] hw/loongarch: Add support loongson3 virt machine type.
On 3/28/22 06:57, Xiaojuan Yang wrote:
+static uint64_t loongarch_qemu_read(void *opaque, hwaddr addr, unsigned size)
+{
+uint64_t feature = 0UL;
+
+switch (addr) {
+case FEATURE_REG:
+feature |= 1UL << IOCSRF_MSI | 1UL << IOCSRF_EXTIOI |
+ 1UL << IOCSRF_CSRIPI;
+return feature ;
What's the point of the feature variable?
+case VENDOR_REG:
+return *(uint64_t *)"Loongson";
+case CPUNAME_REG:
+return *(uint64_t *)"3A5000";
This is definitely wrong, as (1) it depends on host endianness, and (2) you're reading 8
bytes from a 7 byte string.
+static const MemoryRegionOps loongarch_qemu_ops = {
+.read = loongarch_qemu_read,
+.write = loongarch_qemu_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 8,
+},
+.impl = {
+.min_access_size = 4,
+.max_access_size = 8,
+},
The implementation above doesn't actually support access size 4; it only
supports 8.
It doesn't seem like this should be a io region at all, but a ROM.
+static void loongarch_cpu_init(LoongArchCPU *la_cpu, int cpu_num)
+{
+CPULoongArchState *env;
+env = _cpu->env;
+
+memory_region_init_io(>system_iocsr, OBJECT(la_cpu), NULL,
+ env, "iocsr", UINT64_MAX);
+address_space_init(>address_space_iocsr, >system_iocsr, "IOCSR");
+
+timer_init_ns(_cpu->timer, QEMU_CLOCK_VIRTUAL,
+ _constant_timer_cb, la_cpu);
This timer belongs to the cpu, not the board model.
This init belongs over in target/loongarch/.
r~
[PATCH for-7.0 v5] qemu-binfmt-conf.sh: mips: allow nonzero EI_ABIVERSION, distinguish o32 and n32
With the command line flag -mplt and a recent toolchain, ELF binaries generated by gcc can obtain EI_ABIVERSION=1, which makes, e.g., gcc three-stage bootstrap in a mips-unknown-linux-gnu qemu-user chroot fail since the binfmt-misc magic does not match anymore. Also other values are technically possible. qemu executes these binaries just fine, so relax the mask for the EI_ABIVERSION byte at offset 0x08. In addition, extend magic string to distinguish mips o32 and n32 ABI. This information is given by the EF_MIPS_ABI2 (0x20) bit in the e_flags field of the ELF header (a 4-byte value at offset 0x24 for the here applicable ELFCLASS32). See-also: ace3d65459 Signed-off-by: Andreas K. Hüttel Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: WANG Xuerui Cc: Laurent Vivier Cc: WANG Xuerui Cc: Richard Henderson Cc: Alex Bennee Cc: Philippe Mathieu-Daudé Closes: https://gitlab.com/qemu-project/qemu/-/issues/843 --- v5: Fully relax mask for EI_ABIVERSION for all of mips; squash patches since they touch the same lines v4: Unchanged repost of v3 v3: Add the magic extension to distinguish n32 and o32 v2: Add the same EI_ABIVERSION fix for little endian as for big endian v1: Initial version, only handling EI_ABIVERSION=1 on BE scripts/qemu-binfmt-conf.sh | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh index e9bfeb94d3..9cb723f443 100755 --- a/scripts/qemu-binfmt-conf.sh +++ b/scripts/qemu-binfmt-conf.sh @@ -60,28 +60,28 @@ m68k_family=m68k # FIXME: We could use the other endianness on a MIPS host. -mips_magic='\x7fELF\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08' -mips_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff' +mips_magic='\x7fELF\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' +mips_mask='\xff\xff\xff\xff\xff\xff\xff\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20' mips_family=mips -mipsel_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00' -mipsel_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff' +mipsel_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' +mipsel_mask='\xff\xff\xff\xff\xff\xff\xff\x00\x00\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20\x00\x00\x00' mipsel_family=mips -mipsn32_magic='\x7fELF\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08' -mipsn32_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff' +mipsn32_magic='\x7fELF\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20' +mipsn32_mask='\xff\xff\xff\xff\xff\xff\xff\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20' mipsn32_family=mips -mipsn32el_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00' -mipsn32el_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff' +mipsn32el_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20\x00\x00\x00' +mipsn32el_mask='\xff\xff\xff\xff\xff\xff\xff\x00\x00\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff\xff\xff\xff\xff\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x20\x00\x00\x00' mipsn32el_family=mips mips64_magic='\x7fELF\x02\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08' -mips64_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff' +mips64_mask='\xff\xff\xff\xff\xff\xff\xff\x00\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff' mips64_family=mips mips64el_magic='\x7fELF\x02\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x08\x00' -mips64el_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff' +mips64el_mask='\xff\xff\xff\xff\xff\xff\xff\x00\x00\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff' mips64el_family=mips sh4_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x2a\x00' -- 2.34.1
Re: [RFC PATCH v7 18/29] hw/intc: Add LoongArch ls7a msi interrupt controller support(PCH-MSI)
On 28/03/2022 13:57, Xiaojuan Yang wrote:
This patch realize PCH-MSI interrupt controller.
Signed-off-by: Xiaojuan Yang
Signed-off-by: Song Gao
---
hw/intc/Kconfig | 5 ++
hw/intc/loongarch_pch_msi.c | 75 +
hw/intc/meson.build | 1 +
hw/intc/trace-events| 3 ++
hw/loongarch/Kconfig| 1 +
include/hw/intc/loongarch_pch_msi.h | 21
6 files changed, 106 insertions(+)
create mode 100644 hw/intc/loongarch_pch_msi.c
create mode 100644 include/hw/intc/loongarch_pch_msi.h
diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 1fbba2e728..71c04c328e 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -91,3 +91,8 @@ config LOONGARCH_IPI
config LOONGARCH_PCH_PIC
bool
select UNIMP
+
+config LOONGARCH_PCH_MSI
+select MSI_NONBROKEN
+bool
+select UNIMP
diff --git a/hw/intc/loongarch_pch_msi.c b/hw/intc/loongarch_pch_msi.c
new file mode 100644
index 00..57a894f3e5
--- /dev/null
+++ b/hw/intc/loongarch_pch_msi.c
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * QEMU Loongson 7A1000 msi interrupt controller.
+ *
+ * Copyright (C) 2021 Loongson Technology Corporation Limited
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/irq.h"
+#include "hw/intc/loongarch_pch_msi.h"
+#include "hw/intc/loongarch_pch_pic.h"
+#include "hw/pci/msi.h"
+#include "hw/misc/unimp.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+static uint64_t loongarch_msi_mem_read(void *opaque, hwaddr addr, unsigned
size)
+{
+return 0;
+}
+
+static void loongarch_msi_mem_write(void *opaque, hwaddr addr,
+uint64_t val, unsigned size)
+{
+LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(opaque);
+int irq_num = val & 0xff;
+
+trace_loongarch_msi_set_irq(irq_num);
+qemu_set_irq(s->pch_msi_irq[irq_num - PCH_PIC_IRQ_NUM], 1);
+}
+
+static const MemoryRegionOps loongarch_pch_msi_ops = {
+.read = loongarch_msi_mem_read,
+.write = loongarch_msi_mem_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void pch_msi_irq_handler(void *opaque, int irq, int level)
+{
+LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(opaque);
+
+qemu_set_irq(s->pch_msi_irq[irq], level);
+}
+
+static void loongarch_pch_msi_init(Object *obj)
+{
+LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(obj);
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+int i;
+
+memory_region_init_io(>msi_mmio, obj, _pch_msi_ops,
+ s, TYPE_LOONGARCH_PCH_MSI, 0x8);
+sysbus_init_mmio(sbd, >msi_mmio);
+msi_nonbroken = true;
+
+for (i = 0; i < PCH_MSI_IRQ_NUM; i++) {
+sysbus_init_irq(sbd, >pch_msi_irq[i]);
+}
+qdev_init_gpio_in(DEVICE(obj), pch_msi_irq_handler, PCH_MSI_IRQ_NUM);
+}
+
+static const TypeInfo loongarch_pch_msi_info = {
+.name = TYPE_LOONGARCH_PCH_MSI,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(LoongArchPCHMSI),
+.instance_init = loongarch_pch_msi_init,
+};
+
+static void loongarch_pch_msi_register_types(void)
+{
+type_register_static(_pch_msi_info);
+}
+
+type_init(loongarch_pch_msi_register_types)
diff --git a/hw/intc/meson.build b/hw/intc/meson.build
index 960ce81a92..77a30cec33 100644
--- a/hw/intc/meson.build
+++ b/hw/intc/meson.build
@@ -64,3 +64,4 @@ specific_ss.add(when: 'CONFIG_GOLDFISH_PIC', if_true:
files('goldfish_pic.c'))
specific_ss.add(when: 'CONFIG_M68K_IRQC', if_true: files('m68k_irqc.c'))
specific_ss.add(when: 'CONFIG_LOONGARCH_IPI', if_true:
files('loongarch_ipi.c'))
specific_ss.add(when: 'CONFIG_LOONGARCH_PCH_PIC', if_true:
files('loongarch_pch_pic.c'))
+specific_ss.add(when: 'CONFIG_LOONGARCH_PCH_MSI', if_true:
files('loongarch_pch_msi.c'))
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 8c12bdd89f..7c02f8d5f0 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -288,3 +288,6 @@ loongarch_pch_pic_high_readw(unsigned size, uint32_t addr,
unsigned long val) "s
loongarch_pch_pic_high_writew(unsigned size, uint32_t addr, unsigned long val) "size: %u
addr: 0x%"PRIx32 "val: 0x%" PRIx64
loongarch_pch_pic_readb(unsigned size, uint32_t addr, unsigned long val) "size: %u addr:
0x%"PRIx32 "val: 0x%" PRIx64
loongarch_pch_pic_writeb(unsigned size, uint32_t addr, unsigned long val) "size: %u addr:
0x%"PRIx32 "val: 0x%" PRIx64
+
+# loongarch_pch_msi.c
+loongarch_msi_set_irq(int irq_num) "set msi irq %d"
diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig
index 2df45f7e8f..d814fc6103 100644
--- a/hw/loongarch/Kconfig
+++ b/hw/loongarch/Kconfig
@@ -4,3 +4,4 @@ config LOONGARCH_VIRT
select PCI_EXPRESS_GENERIC_BRIDGE
select LOONGARCH_IPI
select LOONGARCH_PCH_PIC
+select LOONGARCH_PCH_MSI
diff --git a/include/hw/intc/loongarch_pch_msi.h
b/include/hw/intc/loongarch_pch_msi.h
new file mode 100644
index 00..68009d4b4a
--- /dev/null
+++
Re: [RFC PATCH v7 11/29] target/loongarch: Add LoongArch interrupt and exception handle
On 3/28/22 06:57, Xiaojuan Yang wrote: 1.This patch Add loongarch interrupt and exception handle. 2.Rename the user excp to the exccode from the csr defintions. Signed-off-by: Xiaojuan Yang Signed-off-by: Song Gao --- linux-user/loongarch64/cpu_loop.c | 8 +- target/loongarch/cpu.c| 260 +- target/loongarch/cpu.h| 11 - target/loongarch/fpu_helper.c | 2 +- target/loongarch/insn_trans/trans_extra.c.inc | 4 +- target/loongarch/translate.c | 2 +- 6 files changed, 261 insertions(+), 26 deletions(-) To repeat my response to the cover letter, the changes in this patch should be folded back into the original patches defining the base architecture. r~
Re: [RFC PATCH v7 13/29] target/loongarch: Add gdb support.
On 3/28/22 06:57, Xiaojuan Yang wrote:
+int loongarch_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
+{
+LoongArchCPU *cpu = LOONGARCH_CPU(cs);
+CPULoongArchState *env = >env;
+target_ulong tmp = ldtul_p(mem_buf);
+
+if (0 <= n && n < 32) {
+return env->gpr[n] = tmp, sizeof(target_ulong);
Ew. Do not use the comma operator with return.
Split these into two statements, all through this file.
r~
Re: [PATCH v5 00/13] KVM: mm: fd-based approach for supporting KVM guest private memory
On Thu, Mar 10, 2022 at 6:09 AM Chao Peng wrote: > > This is the v5 of this series which tries to implement the fd-based KVM > guest private memory. The patches are based on latest kvm/queue branch > commit: > > d5089416b7fb KVM: x86: Introduce KVM_CAP_DISABLE_QUIRKS2 Can this series be run and a VM booted without TDX? A feature like that might help push it forward. --Andy
Re: [RFC PATCH v7 12/29] target/loongarch: Add timer related instructions support.
On 3/28/22 06:57, Xiaojuan Yang wrote:
+#ifndef CONFIG_USER_ONLY
+static bool gen_rdtime(DisasContext *ctx, arg_rr *a,
+ bool word, bool high)
+{
+TCGv dst1 = gpr_dst(ctx, a->rd, EXT_NONE);
+TCGv dst2 = gpr_dst(ctx, a->rj, EXT_NONE);
+
+if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+gen_io_start();
+}
+gen_helper_rdtime_d(dst1, cpu_env);
+if (word) {
+tcg_gen_sextract_tl(dst1, dst1, high ? 32 : 0, 32);
+}
+tcg_gen_ld_i64(dst2, cpu_env, offsetof(CPULoongArchState, CSR_TID));
+
+return true;
+}
Remove all of the ifdefs.
static bool trans_rdtimel_w(DisasContext *ctx, arg_rdtimel_w *a)
{
+#ifdef CONFIG_USER_ONLY
tcg_gen_movi_tl(cpu_gpr[a->rd], 0);
return true;
This (and all of the others) turns out to be a bug, as it fails to write to rj
at all.
+uint64_t helper_rdtime_d(CPULoongArchState *env)
+{
+ LoongArchCPU *cpu = LOONGARCH_CPU(env_cpu(env));
+ return cpu_loongarch_get_constant_timer_counter(cpu);
+}
Here, you could have
#ifdef CONFIG_USER_ONLY
return cpu_get_host_ticks();
#else
...
which is the fallback we use for other targets in user-mode.
You seem to be missing the checks on CSR.MISC.DRDTL* which would raise IPE.
r~
Re: [RFC PATCH v7 16/29] hw/loongarch: Add LoongArch ipi interrupt support(IPI)
On 28/03/2022 13:57, Xiaojuan Yang wrote:
This patch realize the IPI interrupt controller.
Signed-off-by: Xiaojuan Yang
Signed-off-by: Song Gao
---
MAINTAINERS | 2 +
hw/intc/Kconfig | 3 +
hw/intc/loongarch_ipi.c | 164
hw/intc/meson.build | 1 +
hw/intc/trace-events| 4 +
hw/loongarch/Kconfig| 1 +
include/hw/intc/loongarch_ipi.h | 47 +
7 files changed, 222 insertions(+)
create mode 100644 hw/intc/loongarch_ipi.c
create mode 100644 include/hw/intc/loongarch_ipi.h
diff --git a/MAINTAINERS b/MAINTAINERS
index a794f41913..d83b90b5c5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1133,6 +1133,8 @@ F: configs/devices/loongarch64-softmmu/default.mak
F: gdb-xml/loongarch*.xml
F: hw/loongarch/
F: include/hw/loongarch/loongarch.h
+F: include/hw/intc/loongarch_*.h
+F: hw/intc/loongarch_*.c
M68K Machines
-
diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index a7cf301eab..6c7e82da64 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -84,3 +84,6 @@ config GOLDFISH_PIC
config M68K_IRQC
bool
+
+config LOONGARCH_IPI
+bool
diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
new file mode 100644
index 00..89e9019112
--- /dev/null
+++ b/hw/intc/loongarch_ipi.c
@@ -0,0 +1,164 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * LoongArch ipi interrupt support
+ *
+ * Copyright (C) 2021 Loongson Technology Corporation Limited
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/intc/loongarch_ipi.h"
+#include "hw/irq.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "exec/address-spaces.h"
+#include "hw/loongarch/loongarch.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+static uint64_t loongarch_ipi_readl(void *opaque, hwaddr addr, unsigned size)
+{
+IPICore *s = opaque;
+uint64_t ret = 0;
+int index = 0;
+
+addr &= 0xff;
+switch (addr) {
+case CORE_STATUS_OFF:
+ret = s->status;
+break;
+case CORE_EN_OFF:
+ret = s->en;
+break;
+case CORE_SET_OFF:
+ret = 0;
+break;
+case CORE_CLEAR_OFF:
+ret = 0;
+break;
+case CORE_BUF_20 ... CORE_BUF_38 + 4:
+index = (addr - CORE_BUF_20) >> 2;
+ret = s->buf[index];
+break;
+case IOCSR_IPI_SEND:
+ret = s->status;
+break;
+default:
+qemu_log_mask(LOG_UNIMP, "invalid read: %x", (uint32_t)addr);
+break;
+}
+
+trace_loongarch_ipi_read(size, (uint64_t)addr, ret);
+return ret;
+}
+
+static void loongarch_ipi_writel(void *opaque, hwaddr addr, uint64_t val,
+ unsigned size)
+{
+IPICore *s = opaque;
+int index = 0;
+
+addr &= 0xff;
+trace_loongarch_ipi_write(size, (uint64_t)addr, val);
+switch (addr) {
+case CORE_STATUS_OFF:
+qemu_log_mask(LOG_GUEST_ERROR, "can not be written");
+break;
+case CORE_EN_OFF:
+s->en = val;
+break;
+case CORE_SET_OFF:
+s->status |= val;
+if (s->status != 0) {
+qemu_irq_raise(s->irq);
+}
+break;
+case CORE_CLEAR_OFF:
+s->status ^= val;
+if (s->status == 0) {
+qemu_irq_lower(s->irq);
+}
+break;
+case CORE_BUF_20 ... CORE_BUF_38 + 4:
+index = (addr - CORE_BUF_20) >> 2;
+s->buf[index] = val;
+break;
+case IOCSR_IPI_SEND:
+s->status |= val;
+break;
+default:
+qemu_log_mask(LOG_UNIMP, "invalid write: %x", (uint32_t)addr);
+break;
+}
+}
+
+static const MemoryRegionOps loongarch_ipi_ops = {
+.read = loongarch_ipi_readl,
+.write = loongarch_ipi_writel,
+.impl.min_access_size = 4,
+.impl.max_access_size = 4,
+.valid.min_access_size = 4,
+.valid.max_access_size = 8,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void loongarch_ipi_init(Object *obj)
+{
+LoongArchIPI *s = LOONGARCH_IPI(obj);
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+int cpu;
+
+for (cpu = 0; cpu < MAX_IPI_CORE_NUM; cpu++) {
+memory_region_init_io(>ipi_mmio[cpu], obj, _ipi_ops,
+ >core[cpu], "loongarch_ipi", 0x100);
+sysbus_init_mmio(sbd, >ipi_mmio[cpu]);
+qdev_init_gpio_out(DEVICE(obj), >core[cpu].irq, 1);
+ }
+}
+
+static const VMStateDescription vmstate_ipi_core = {
+.name = "ipi-single",
+.version_id = 0,
+.minimum_version_id = 0,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(status, IPICore),
+VMSTATE_UINT32(en, IPICore),
+VMSTATE_UINT32(set, IPICore),
+VMSTATE_UINT32(clear, IPICore),
+VMSTATE_UINT32_ARRAY(buf, IPICore, MAX_IPI_MBX_NUM * 2),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static const VMStateDescription vmstate_loongarch_ipi = {
+.name =
Re: [RFC PATCH v7 10/29] target/loongarch: Add other core instructions support
On 3/28/22 06:57, Xiaojuan Yang wrote:
+void helper_idle(CPULoongArchState *env)
+{
+CPUState *cs = env_cpu(env);
+
+cs->halted = 1;
+cpu_reset_interrupt(cs, CPU_INTERRUPT_WAKE);
+do_raise_exception(env, EXCP_HLT, 0);
+}
Why are you messing with CPU_INTERRUPT_WAKE?
You only ever reset it, and never set it.
r~
Re: [RFC PATCH v7 19/29] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)
On 28/03/2022 13:57, Xiaojuan Yang wrote:
This patch realize the EIOINTC interrupt controller.
Signed-off-by: Xiaojuan Yang
Signed-off-by: Song Gao
---
hw/intc/Kconfig| 3 +
hw/intc/loongarch_extioi.c | 408 +
hw/intc/meson.build| 1 +
hw/intc/trace-events | 11 +
hw/loongarch/Kconfig | 1 +
include/hw/intc/loongarch_extioi.h | 77 ++
6 files changed, 501 insertions(+)
create mode 100644 hw/intc/loongarch_extioi.c
create mode 100644 include/hw/intc/loongarch_extioi.h
diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 71c04c328e..28bd1f185d 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -96,3 +96,6 @@ config LOONGARCH_PCH_MSI
select MSI_NONBROKEN
bool
select UNIMP
+
+config LOONGARCH_EXTIOI
+bool
diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
new file mode 100644
index 00..af28e8d6e9
--- /dev/null
+++ b/hw/intc/loongarch_extioi.c
@@ -0,0 +1,408 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Loongson 3A5000 ext interrupt controller emulation
+ *
+ * Copyright (C) 2021 Loongson Technology Corporation Limited
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qemu/log.h"
+#include "hw/irq.h"
+#include "hw/sysbus.h"
+#include "hw/loongarch/loongarch.h"
+#include "hw/qdev-properties.h"
+#include "exec/address-spaces.h"
+#include "hw/intc/loongarch_extioi.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+static void extioi_update_irq(void *opaque, int irq_num, int level)
+{
+LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+uint8_t ipnum, cpu;
+unsigned long found1, found2;
+
+ipnum = s->sw_ipmap[irq_num];
+cpu = s->sw_coremap[irq_num];
+if (level == 1) {
+if (test_bit(irq_num, (void *)s->enable) == false) {
+return;
+}
+bitmap_set((void *)s->coreisr[cpu], irq_num, 1);
+found1 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
+ EXTIOI_IRQS, 0);
+bitmap_set((void *)&(s->sw_ipisr[cpu][ipnum]), irq_num, 1);
+
+if (found1 >= EXTIOI_IRQS) {
+qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+}
+} else {
+bitmap_clear((void *)s->coreisr[cpu], irq_num, 1);
+found1 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
+ EXTIOI_IRQS, 0);
+bitmap_clear((void *)&(s->sw_ipisr[cpu][ipnum]), irq_num, 1);
+found2 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
+ EXTIOI_IRQS, 0);
+
+if ((found1 < EXTIOI_IRQS) && (found2 >= EXTIOI_IRQS)) {
+qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+}
+}
+}
+
+static void extioi_setirq(void *opaque, int irq, int level)
+{
+LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+trace_extioi_setirq(irq, level);
+extioi_update_irq(s, irq, level);
+}
+
+static uint64_t extioi_nodetype_readw(void *opaque, hwaddr addr, unsigned size)
+{
+LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+unsigned long offset = addr & 0x;
+uint32_t ret, index;
+
+switch (offset) {
+case EXTIOI_NODETYPE_START ... EXTIOI_NODETYPE_END - 1:
+index = (offset - EXTIOI_NODETYPE_START) >> 2;
+ret = s->nodetype[index];
+break;
+default:
+break;
+}
+
+trace_loongarch_extioi_nodetype_readw((uint32_t)addr, ret);
+return ret;
+}
+
+static void extioi_nodetype_writew(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size)
+{
+LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+int index;
+uint32_t offset;
+trace_loongarch_extioi_nodetype_writew(size, (uint32_t)addr, val);
+
+offset = addr & 0x;
+
+switch (offset) {
+case EXTIOI_NODETYPE_START ... EXTIOI_NODETYPE_END - 1:
+index = (offset - EXTIOI_NODETYPE_START) >> 2;
+s->nodetype[index] = val;
+break;
+default:
+break;
+}
+}
+
+static uint64_t extioi_ipmap_enable_read(void *opaque, hwaddr addr,
+ unsigned size)
+{
+LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+uint8_t ret;
+
+switch (addr) {
+case EXTIOI_IPMAP_START ... EXTIOI_IPMAP_END - 1:
+ret = s->ipmap[addr];
+break;
+case EXTIOI_ENABLE_START ... EXTIOI_ENABLE_END - 1:
+addr -= EXTIOI_ENABLE_START;
+ret = s->enable[addr];
+break;
+default:
+break;
+}
+
+trace_loongarch_extioi_ipmap_enable_read((uint8_t)addr, ret);
+return ret;
+}
+
+static void extioi_ipmap_enable_write(void *opaque, hwaddr addr,
+ uint64_t value, unsigned size)
+{
+LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+uint8_t old_data, val = value & 0xff;
+int i, level, ipnum, irqnum;
+
[RFC PATCH 4/6] softfloat: add float*_to_int128 conversion methods
From: Matheus Ferst
Implements parts_float_to_int2 based on parts_float_to_int logic. The
new methods return the lower part of the result through the "lo"
pointer.
Signed-off-by: Matheus Ferst
---
fpu/softfloat-parts.c.inc | 75 +++
fpu/softfloat.c | 50 ++
include/fpu/softfloat.h | 8 +
3 files changed, 133 insertions(+)
diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
index 2767aeac03..344df52e5f 100644
--- a/fpu/softfloat-parts.c.inc
+++ b/fpu/softfloat-parts.c.inc
@@ -1096,6 +1096,81 @@ static int64_t partsN(float_to_sint)(FloatPartsN *p,
FloatRoundMode rmode,
return r;
}
+static int64_t partsN(float_to_sint2)(FloatPartsN *p, FloatRoundMode rmode,
+ int scale, float_status *s, uint64_t *lo)
+{
+int flags = 0;
+uint64_t hi;
+
+switch (p->cls) {
+case float_class_snan:
+flags |= float_flag_invalid_snan;
+/* fall through */
+case float_class_qnan:
+flags |= float_flag_invalid;
+hi = UINT64_MAX;
+*lo = UINT64_MAX;
+break;
+
+case float_class_inf:
+flags = float_flag_invalid | float_flag_invalid_cvti;
+if (p->sign) {
+hi = INT64_MIN;
+*lo = 0;
+} else {
+hi = INT64_MAX;
+*lo = UINT64_MAX;
+}
+break;
+
+case float_class_zero:
+*lo = 0;
+return 0;
+
+case float_class_normal:
+if (parts_round_to_int_normal(p, rmode, scale, N - 2)) {
+flags = float_flag_inexact;
+}
+
+if (p->exp <= DECOMPOSED_BINARY_POINT) {
+hi = 0;
+*lo = p->frac_hi >> (DECOMPOSED_BINARY_POINT - p->exp);
+} else if (p->exp <= 127) {
+int shift = 127 - p->exp;
+hi = shr_double(0, p->frac_hi, shift);
+if (N > 64) {
+*lo = shr_double(p->frac_hi, p->frac_lo, shift);
+} else {
+*lo = shr_double(p->frac_hi, 0, shift);
+}
+} else {
+hi = UINT64_MAX;
+*lo = UINT64_MAX;
+}
+if (p->sign) {
+if (hi < INT64_MIN || (hi == INT64_MIN && *lo == 0)) {
+*lo = -*lo;
+hi = ~hi + !*lo;
+} else {
+flags = float_flag_invalid | float_flag_invalid_cvti;
+hi = INT64_MIN;
+*lo = 0;
+}
+} else if (hi > INT64_MAX) {
+flags = float_flag_invalid | float_flag_invalid_cvti;
+hi = INT64_MAX;
+*lo = UINT64_MAX;
+}
+break;
+
+default:
+g_assert_not_reached();
+}
+
+float_raise(flags, s);
+return hi;
+}
+
/*
* Returns the result of converting the floating-point value `a' to
* the unsigned integer format. The conversion is performed according
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index fe4320060c..41a18f86df 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -840,6 +840,15 @@ static int64_t parts128_float_to_sint(FloatParts128 *p,
FloatRoundMode rmode,
#define parts_float_to_sint(P, R, Z, MN, MX, S) \
PARTS_GENERIC_64_128(float_to_sint, P)(P, R, Z, MN, MX, S)
+static int64_t parts64_float_to_sint2(FloatParts64 *p, FloatRoundMode rmode,
+ int scale, float_status *s, uint64_t
*lo);
+static int64_t parts128_float_to_sint2(FloatParts128 *p, FloatRoundMode rmode,
+ int scale, float_status *s,
+ uint64_t *lo);
+
+#define parts_float_to_sint2(P, R, Z, S, H) \
+PARTS_GENERIC_64_128(float_to_sint2, P)(P, R, Z, S, H)
+
static uint64_t parts64_float_to_uint(FloatParts64 *p, FloatRoundMode rmode,
int scale, uint64_t max,
float_status *s);
@@ -3135,6 +3144,15 @@ int64_t float64_to_int64_scalbn(float64 a,
FloatRoundMode rmode, int scale,
return parts_float_to_sint(, rmode, scale, INT64_MIN, INT64_MAX, s);
}
+int64_t float64_to_int128_scalbn(float64 a, FloatRoundMode rmode, int scale,
+ float_status *s, uint64_t *lo)
+{
+FloatParts64 p;
+
+float64_unpack_canonical(, a, s);
+return parts_float_to_sint2(, rmode, scale, s, lo);
+}
+
int16_t bfloat16_to_int16_scalbn(bfloat16 a, FloatRoundMode rmode, int scale,
float_status *s)
{
@@ -3180,6 +3198,16 @@ static int64_t float128_to_int64_scalbn(float128 a,
FloatRoundMode rmode,
return parts_float_to_sint(, rmode, scale, INT64_MIN, INT64_MAX, s);
}
+static int64_t float128_to_int128_scalbn(float128 a, FloatRoundMode rmode,
+ int scale, float_status *s,
+ uint64_t *lo)
+{
+FloatParts128 p;
+
+float128_unpack_canonical(, a,
[RFC PATCH 6/6] target/ppc: implement xscvqp[su]qz
From: Matheus Ferst
Implement the following PowerISA v3.1 instructions:
xscvqpsqz: VSX Scalar Convert with round to zero Quad-Precision to
Signed Quadword
xscvqpuqz: VSX Scalar Convert with round to zero Quad-Precision to
Unsigned Quadword
Signed-off-by: Matheus Ferst
---
target/ppc/fpu_helper.c | 23 +++
target/ppc/helper.h | 2 ++
target/ppc/insn32.decode| 2 ++
target/ppc/translate/vsx-impl.c.inc | 2 ++
4 files changed, 29 insertions(+)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 5101ba92ae..712306b5a1 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -2925,6 +2925,29 @@ VSX_CVT_FP_TO_INT(xvcvspsxws, 4, float32, int32,
VsrW(i), VsrW(i), 0x8000U)
VSX_CVT_FP_TO_INT(xvcvspuxds, 2, float32, uint64, VsrW(2 * i), VsrD(i), 0ULL)
VSX_CVT_FP_TO_INT(xvcvspuxws, 4, float32, uint32, VsrW(i), VsrW(i), 0U)
+#define VSX_CVT_FP_TO_INT128(op, tp, rnan) \
+void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb)\
+{ \
+ppc_vsr_t t;\
+int flags; \
+\
+helper_reset_fpstatus(env); \
+t.VsrD(0) = float128_to_##tp##_round_to_zero(xb->f128, >fp_status, \
+ (1)); \
+flags = get_float_exception_flags(>fp_status); \
+if (unlikely(flags & float_flag_invalid)) { \
+t.VsrD(0) = float_invalid_cvt(env, flags, t.VsrD(0), rnan, 0, \
+ GETPC()); \
+t.VsrD(1) = -(t.VsrD(0) & 1); \
+} \
+\
+*xt = t;\
+do_float_check_status(env, GETPC());\
+}
+
+VSX_CVT_FP_TO_INT128(XSCVQPUQZ, uint128, 0)
+VSX_CVT_FP_TO_INT128(XSCVQPSQZ, int128, 0x8000ULL);
+
/*
* Likewise, except that the result is duplicated into both subwords.
* Power ISA v3.1 has Programming Notes for these insns:
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 7df0c01819..aa6773c4a5 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -388,6 +388,8 @@ DEF_HELPER_4(xscvqpsdz, void, env, i32, vsr, vsr)
DEF_HELPER_4(xscvqpswz, void, env, i32, vsr, vsr)
DEF_HELPER_4(xscvqpudz, void, env, i32, vsr, vsr)
DEF_HELPER_4(xscvqpuwz, void, env, i32, vsr, vsr)
+DEF_HELPER_3(XSCVQPUQZ, void, env, vsr, vsr)
+DEF_HELPER_3(XSCVQPSQZ, void, env, vsr, vsr)
DEF_HELPER_3(XSCVUQQP, void, env, vsr, vsr)
DEF_HELPER_3(XSCVSQQP, void, env, vsr, vsr)
DEF_HELPER_3(xscvhpdp, void, env, vsr, vsr)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 6fb568c1fe..39372fe673 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -695,6 +695,8 @@ XSCMPGTQP 11 . . . 0011100100 - @X
## VSX Binary Floating-Point Convert Instructions
XSCVQPDP11 . 10100 . 1101000100 . @X_tb_rc
+XSCVQPUQZ 11 . 0 . 1101000100 - @X_tb
+XSCVQPSQZ 11 . 01000 . 1101000100 - @X_tb
XSCVUQQP11 . 00011 . 1101000100 - @X_tb
XSCVSQQP11 . 01011 . 1101000100 - @X_tb
XVCVBF16SPN 00 . 1 . 111011011 .. @XX2
diff --git a/target/ppc/translate/vsx-impl.c.inc
b/target/ppc/translate/vsx-impl.c.inc
index a305579ecc..5cc006d715 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -855,6 +855,8 @@ static bool do_helper_env_X_tb(DisasContext *ctx, arg_X_tb
*a,
return true;
}
+TRANS(XSCVQPUQZ, do_helper_env_X_tb, gen_helper_XSCVQPUQZ)
+TRANS(XSCVQPSQZ, do_helper_env_X_tb, gen_helper_XSCVQPSQZ)
TRANS(XSCVUQQP, do_helper_env_X_tb, gen_helper_XSCVUQQP)
TRANS(XSCVSQQP, do_helper_env_X_tb, gen_helper_XSCVSQQP)
--
2.25.1
[RFC PATCH 1/6] softfloat: add uint128_to_float* conversion methods
From: Matheus Ferst
Based on parts_uint_to_float, implements parts_uint_to_float2 that
receives a 128-bit integer through a pair of uint64_t values.
Signed-off-by: Matheus Ferst
---
fpu/softfloat-parts.c.inc | 19 +++
fpu/softfloat.c | 30 ++
include/fpu/softfloat.h | 4
3 files changed, 53 insertions(+)
diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
index db3e1f393d..0bbecf835f 100644
--- a/fpu/softfloat-parts.c.inc
+++ b/fpu/softfloat-parts.c.inc
@@ -1219,6 +1219,25 @@ static void partsN(uint_to_float)(FloatPartsN *p,
uint64_t a,
}
}
+static void partsN(uint_to_float2)(FloatPartsN *p, uint64_t hi, uint64_t lo,
+ int scale, float_status *status)
+{
+
+if (hi == 0) {
+parts_uint_to_float(p, lo, scale, status);
+} else {
+int shift = clz64(hi);
+memset(p, 0, sizeof(*p));
+scale = MIN(MAX(scale, -0x1), 0x1);
+p->cls = float_class_normal;
+p->exp = 127 - shift + scale;
+p->frac_hi = shl_double(hi, lo, shift);
+if (N > 64) {
+p->frac_lo = shl_double(lo, 0, shift);
+}
+}
+}
+
/*
* Float min/max.
*/
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 7f524d4377..980ddfe5a1 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -866,6 +866,14 @@ static void parts128_uint_to_float(FloatParts128 *p,
uint64_t a,
#define parts_uint_to_float(P, I, Z, S) \
PARTS_GENERIC_64_128(uint_to_float, P)(P, I, Z, S)
+static void parts64_uint_to_float2(FloatParts64 *p, uint64_t hi, uint64_t lo,
+ int scale, float_status *s);
+static void parts128_uint_to_float2(FloatParts128 *p, uint64_t hi, uint64_t lo,
+int scale, float_status *s);
+
+#define parts_uint_to_float2(P, H, L, Z, S) \
+PARTS_GENERIC_64_128(uint_to_float2, P)(P, H, L, Z, S)
+
static FloatParts64 *parts64_minmax(FloatParts64 *a, FloatParts64 *b,
float_status *s, int flags);
static FloatParts128 *parts128_minmax(FloatParts128 *a, FloatParts128 *b,
@@ -3888,6 +3896,15 @@ float32 uint16_to_float32(uint16_t a, float_status
*status)
return uint64_to_float32_scalbn(a, 0, status);
}
+float64 uint128_to_float64_scalbn(uint64_t hi, uint64_t lo, int scale,
+ float_status *status)
+{
+FloatParts64 p;
+
+parts_uint_to_float2(, hi, lo, scale, status);
+return float64_round_pack_canonical(, status);
+}
+
float64 uint64_to_float64_scalbn(uint64_t a, int scale, float_status *status)
{
FloatParts64 p;
@@ -3913,6 +3930,11 @@ float64 uint16_to_float64_scalbn(uint16_t a, int scale,
float_status *status)
return uint64_to_float64_scalbn(a, scale, status);
}
+float64 uint128_to_float64(uint64_t hi, uint64_t lo, float_status *status)
+{
+return uint128_to_float64_scalbn(hi, lo, 0, status);
+}
+
float64 uint64_to_float64(uint64_t a, float_status *status)
{
return uint64_to_float64_scalbn(a, 0, status);
@@ -3969,6 +3991,14 @@ float128 uint64_to_float128(uint64_t a, float_status
*status)
return float128_round_pack_canonical(, status);
}
+float128 uint128_to_float128(uint64_t hi, uint64_t lo, float_status *status)
+{
+FloatParts128 p;
+
+parts128_uint_to_float2(, hi, lo, 0, status);
+return float128_round_pack_canonical(, status);
+}
+
/*
* Minimum and maximum
*/
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index d34b2c44d2..2ef4fec3d0 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -169,6 +169,8 @@ float64 int64_to_float64_scalbn(int64_t, int, float_status
*status);
float64 uint16_to_float64_scalbn(uint16_t, int, float_status *status);
float64 uint32_to_float64_scalbn(uint32_t, int, float_status *status);
float64 uint64_to_float64_scalbn(uint64_t, int, float_status *status);
+float64 uint128_to_float64_scalbn(uint64_t, uint64_t, int,
+ float_status *status);
float64 int16_to_float64(int16_t, float_status *status);
float64 int32_to_float64(int32_t, float_status *status);
@@ -176,6 +178,7 @@ float64 int64_to_float64(int64_t, float_status *status);
float64 uint16_to_float64(uint16_t, float_status *status);
float64 uint32_to_float64(uint32_t, float_status *status);
float64 uint64_to_float64(uint64_t, float_status *status);
+float64 uint128_to_float64(uint64_t, uint64_t, float_status *status);
floatx80 int32_to_floatx80(int32_t, float_status *status);
floatx80 int64_to_floatx80(int64_t, float_status *status);
@@ -183,6 +186,7 @@ floatx80 int64_to_floatx80(int64_t, float_status *status);
float128 int32_to_float128(int32_t, float_status *status);
float128 int64_to_float128(int64_t, float_status *status);
float128 uint64_to_float128(uint64_t, float_status *status);
+float128 uint128_to_float128(uint64_t, uint64_t, float_status *status);
[RFC PATCH 5/6] target/ppc: implement xscv[su]qqp
From: Matheus Ferst
Implement the following PowerISA v3.1 instructions:
xscvsqqp: VSX Scalar Convert with round Signed Quadword to
Quad-Precision
xscvuqqp: VSX Scalar Convert with round Unsigned Quadword to
Quad-Precision format
Signed-off-by: Matheus Ferst
---
target/ppc/fpu_helper.c | 11 +++
target/ppc/helper.h | 2 ++
target/ppc/insn32.decode| 5 +
target/ppc/translate/vsx-impl.c.inc | 20
4 files changed, 38 insertions(+)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 7e8be99cc0..5101ba92ae 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -3058,6 +3058,17 @@ void helper_##op(CPUPPCState *env, ppc_vsr_t *xt,
ppc_vsr_t *xb)\
VSX_CVT_INT_TO_FP2(xvcvsxdsp, int64, float32)
VSX_CVT_INT_TO_FP2(xvcvuxdsp, uint64, float32)
+#define VSX_CVT_INT128_TO_FP(op, tp)\
+void helper_##op(CPUPPCState *env, ppc_vsr_t *xt, ppc_vsr_t *xb)\
+{ \
+xt->f128 = tp##_to_float128(xb->VsrD(0), xb->VsrD(1), >fp_status); \
+helper_compute_fprf_float128(env, xt->f128);\
+do_float_check_status(env, GETPC());\
+}
+
+VSX_CVT_INT128_TO_FP(XSCVUQQP, uint128);
+VSX_CVT_INT128_TO_FP(XSCVSQQP, int128);
+
/*
* VSX_CVT_INT_TO_FP_VECTOR - VSX integer to floating point conversion
* op- instruction mnemonic
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 57da11c77e..7df0c01819 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -388,6 +388,8 @@ DEF_HELPER_4(xscvqpsdz, void, env, i32, vsr, vsr)
DEF_HELPER_4(xscvqpswz, void, env, i32, vsr, vsr)
DEF_HELPER_4(xscvqpudz, void, env, i32, vsr, vsr)
DEF_HELPER_4(xscvqpuwz, void, env, i32, vsr, vsr)
+DEF_HELPER_3(XSCVUQQP, void, env, vsr, vsr)
+DEF_HELPER_3(XSCVSQQP, void, env, vsr, vsr)
DEF_HELPER_3(xscvhpdp, void, env, vsr, vsr)
DEF_HELPER_4(xscvsdqp, void, env, i32, vsr, vsr)
DEF_HELPER_3(xscvspdp, void, env, vsr, vsr)
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index ac2d3da9a7..6fb568c1fe 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -91,6 +91,9 @@
@X_tp_a_bp_rc .. 0 ra:5 0 .. rc:1 _rc
rt=%x_frtp rb=%x_frbp
+_tb rt rb
+@X_tb .. rt:5 . rb:5 .. . _tb
+
_tb_rcrt rb rc:bool
@X_tb_rc.. rt:5 . rb:5 .. rc:1 _tb_rc
@@ -692,6 +695,8 @@ XSCMPGTQP 11 . . . 0011100100 - @X
## VSX Binary Floating-Point Convert Instructions
XSCVQPDP11 . 10100 . 1101000100 . @X_tb_rc
+XSCVUQQP11 . 00011 . 1101000100 - @X_tb
+XSCVSQQP11 . 01011 . 1101000100 - @X_tb
XVCVBF16SPN 00 . 1 . 111011011 .. @XX2
XVCVSPBF16 00 . 10001 . 111011011 .. @XX2
diff --git a/target/ppc/translate/vsx-impl.c.inc
b/target/ppc/translate/vsx-impl.c.inc
index d1f614..a305579ecc 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -838,6 +838,26 @@ static bool trans_XSCVQPDP(DisasContext *ctx, arg_X_tb_rc
*a)
return true;
}
+static bool do_helper_env_X_tb(DisasContext *ctx, arg_X_tb *a,
+ void (*gen_helper)(TCGv_ptr, TCGv_ptr,
TCGv_ptr))
+{
+TCGv_ptr xt, xb;
+
+REQUIRE_INSNS_FLAGS2(ctx, ISA310);
+REQUIRE_VSX(ctx);
+
+xt = gen_avr_ptr(a->rt);
+xb = gen_avr_ptr(a->rb);
+gen_helper(cpu_env, xt, xb);
+tcg_temp_free_ptr(xt);
+tcg_temp_free_ptr(xb);
+
+return true;
+}
+
+TRANS(XSCVUQQP, do_helper_env_X_tb, gen_helper_XSCVUQQP)
+TRANS(XSCVSQQP, do_helper_env_X_tb, gen_helper_XSCVSQQP)
+
#define GEN_VSX_HELPER_2(name, op1, op2, inval, type) \
static void gen_##name(DisasContext *ctx) \
{ \
--
2.25.1
Re: [RFC PATCH v7 17/29] hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)
On 28/03/2022 13:57, Xiaojuan Yang wrote:
This patch realize the PCH-PIC interrupt controller.
Signed-off-by: Xiaojuan Yang
Signed-off-by: Song Gao
---
hw/intc/Kconfig | 4 +
hw/intc/loongarch_pch_pic.c | 488
hw/intc/meson.build | 1 +
hw/intc/trace-events| 9 +
hw/loongarch/Kconfig| 1 +
include/hw/intc/loongarch_pch_pic.h | 80 +
6 files changed, 583 insertions(+)
create mode 100644 hw/intc/loongarch_pch_pic.c
create mode 100644 include/hw/intc/loongarch_pch_pic.h
diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 6c7e82da64..1fbba2e728 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -87,3 +87,7 @@ config M68K_IRQC
config LOONGARCH_IPI
bool
+
+config LOONGARCH_PCH_PIC
+bool
+select UNIMP
diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c
new file mode 100644
index 00..04b9bdce36
--- /dev/null
+++ b/hw/intc/loongarch_pch_pic.c
@@ -0,0 +1,488 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * QEMU Loongson 7A1000 I/O interrupt controller.
+ *
+ * Copyright (C) 2021 Loongson Technology Corporation Limited
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/loongarch/loongarch.h"
+#include "hw/irq.h"
+#include "hw/intc/loongarch_pch_pic.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+static void pch_pic_update_irq(LoongArchPCHPIC *s, uint32_t mask,
+ int level, int hi)
+{
+uint32_t val, irq;
+
+if (level == 1) {
+if (hi) {
+val = mask & s->intirr_hi & (~s->int_mask_hi);
+irq = find_first_bit((void *), 32);
+if (irq != 32) {
+s->intisr_hi |= 1ULL << irq;
+qemu_set_irq(s->parent_irq[s->htmsi_vector[irq + 32]], 1);
+}
+} else {
+val = mask & s->intirr_lo & (~s->int_mask_lo);
+irq = find_first_bit((void *), 32);
+if (irq != 32) {
+s->intisr_lo |= 1ULL << irq;
+qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 1);
+}
+}
+} else {
+if (hi) {
+val = mask & s->intisr_hi;
+irq = find_first_bit((void *), 32);
+if (irq != 32) {
+s->intisr_hi &= ~(0x1ULL << irq);
+qemu_set_irq(s->parent_irq[s->htmsi_vector[irq + 32]], 0);
+}
+} else {
+val = mask & s->intisr_lo;
+irq = find_first_bit((void *), 32);
+if (irq != 32) {
+s->intisr_lo &= ~(0x1ULL << irq);
+qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 0);
+}
+}
+}
+}
+
+static void pch_pic_irq_handler(void *opaque, int irq, int level)
+{
+LoongArchPCHPIC *s = LOONGARCH_PCH_PIC(opaque);
+int hi = 0;
+uint32_t mask;
+
+assert(irq < PCH_PIC_IRQ_NUM);
+trace_pch_pic_irq_handler(irq, level);
+
+hi = (irq >= 32) ? 1 : 0;
+if (hi) {
+irq = irq - 32;
+}
+
+mask = 1ULL << irq;
+
+if (hi) {
+if (s->intedge_hi & mask) {
+/* Edge triggered */
+if (level) {
+if ((s->last_intirr_hi & mask) == 0) {
+s->intirr_hi |= mask;
+}
+s->last_intirr_hi |= mask;
+} else {
+s->last_intirr_hi &= ~mask;
+}
+} else {
+/* Level triggered */
+if (level) {
+s->intirr_hi |= mask;
+s->last_intirr_hi |= mask;
+} else {
+s->intirr_hi &= ~mask;
+s->last_intirr_hi &= ~mask;
+}
+}
+} else {
+if (s->intedge_lo & mask) {
+/* Edge triggered */
+if (level) {
+if ((s->last_intirr_lo & mask) == 0) {
+s->intirr_lo |= mask;
+}
+s->last_intirr_lo |= mask;
+} else {
+s->last_intirr_lo &= ~mask;
+}
+} else {
+/* Level triggered */
+if (level) {
+s->intirr_lo |= mask;
+s->last_intirr_lo |= mask;
+} else {
+s->intirr_lo &= ~mask;
+s->last_intirr_lo &= ~mask;
+}
+
+}
+}
+pch_pic_update_irq(s, mask, level, hi);
+}
+
+static uint64_t loongarch_pch_pic_low_readw(void *opaque, hwaddr addr,
+unsigned size)
+{
+LoongArchPCHPIC *s = LOONGARCH_PCH_PIC(opaque);
+uint64_t val = 0;
+uint32_t offset = addr & 0xfff;
+
+switch (offset) {
+case PCH_PIC_INT_ID_LO:
+val = PCH_PIC_INT_ID_VAL;
+break;
+case PCH_PIC_INT_ID_HI:
+val = PCH_PIC_INT_ID_NUM;
+break;
+case PCH_PIC_INT_MASK_LO:
+val = s->int_mask_lo;
+
[RFC PATCH 0/6] softfloat 128-bit integer support
From: Matheus Ferst This RFC is a first attempt at implementing the 128-bit integer conversion routines in softfloat, as required by the xscv[su]qqp and xscvqp[su]qz instructions of PowerISA v3.1. Instead of using int128.h, int-to-float routines receive the 128-bit numbers through a pair of 64-bit values, and float-to-int conversions use a pointer to return the lower half of the result. We only need the parts128 methods, but since the difference to parts64 ones seemed minor, I included both in this patch. RFC: - Should we use struct Int128 instead of 64-bit value pairs? - I've not tested the float64 methods since the PPC instructions only use the quad-precision routines. Should we keep them in the final version? Matheus Ferst (6): softfloat: add uint128_to_float* conversion methods softfloat: add int128_to_float* conversion methods softfloat: add float*_to_uint128 conversion methods softfloat: add float*_to_int128 conversion methods target/ppc: implement xscv[su]qqp target/ppc: implement xscvqp[su]qz fpu/softfloat-parts.c.inc | 202 fpu/softfloat.c | 161 ++ include/fpu/softfloat.h | 23 target/ppc/fpu_helper.c | 34 + target/ppc/helper.h | 4 + target/ppc/insn32.decode| 7 + target/ppc/translate/vsx-impl.c.inc | 22 +++ 7 files changed, 453 insertions(+) -- 2.25.1
[RFC PATCH 2/6] softfloat: add int128_to_float* conversion methods
From: Matheus Ferst
Based on parts_sint_to_float, implements parts_sint_to_float2 that
receives a 128-bit signed integer via int64_t and uint64_t arguments.
Signed-off-by: Matheus Ferst
---
fpu/softfloat-parts.c.inc | 37 +
fpu/softfloat.c | 30 ++
include/fpu/softfloat.h | 3 +++
3 files changed, 70 insertions(+)
diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
index 0bbecf835f..5f7f107a0d 100644
--- a/fpu/softfloat-parts.c.inc
+++ b/fpu/softfloat-parts.c.inc
@@ -1196,6 +1196,43 @@ static void partsN(sint_to_float)(FloatPartsN *p,
int64_t a,
p->frac_hi = f << shift;
}
+static void partsN(sint_to_float2)(FloatPartsN *p, int64_t hi, uint64_t lo,
+ int scale, float_status *status)
+{
+uint64_t f = hi;
+int shift;
+
+if (hi == 0) {
+parts_uint_to_float(p, lo, scale, status);
+} else {
+memset(p, 0, sizeof(*p));
+p->cls = float_class_normal;
+if (hi < 0) {
+lo = -lo;
+f = ~f + !lo;
+p->sign = true;
+}
+if (f != 0) {
+shift = clz64(f);
+} else {
+shift = 64 + clz64(lo);
+}
+scale = MIN(MAX(scale, -0x1), 0x1);
+
+p->exp = 127 - shift + scale;
+
+if (shift >= 64) {
+f = lo;
+lo = 0;
+shift -= 64;
+}
+p->frac_hi = shl_double(f, lo, shift);
+if (N > 64) {
+p->frac_lo = shl_double(lo, 0, shift);
+}
+}
+}
+
/*
* Unsigned Integer to float conversions
*
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 980ddfe5a1..ac3d6f5ab0 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -858,6 +858,14 @@ static void parts128_sint_to_float(FloatParts128 *p,
int64_t a,
#define parts_sint_to_float(P, I, Z, S) \
PARTS_GENERIC_64_128(sint_to_float, P)(P, I, Z, S)
+static void parts64_sint_to_float2(FloatParts64 *p, int64_t hi, uint64_t lo,
+ int scale, float_status *s);
+static void parts128_sint_to_float2(FloatParts128 *p, int64_t hi, uint64_t lo,
+int scale, float_status *s);
+
+#define parts_sint_to_float2(P, H, L, Z, S) \
+PARTS_GENERIC_64_128(sint_to_float2, P)(P, H, L, Z, S)
+
static void parts64_uint_to_float(FloatParts64 *p, uint64_t a,
int scale, float_status *s);
static void parts128_uint_to_float(FloatParts128 *p, uint64_t a,
@@ -3715,6 +3723,15 @@ float32 int16_to_float32(int16_t a, float_status *status)
return int64_to_float32_scalbn(a, 0, status);
}
+float64 int128_to_float64_scalbn(int64_t hi, uint64_t lo, int scale,
+ float_status *status)
+{
+FloatParts64 p;
+
+parts_sint_to_float2(, hi, lo, scale, status);
+return float64_round_pack_canonical(, status);
+}
+
float64 int64_to_float64_scalbn(int64_t a, int scale, float_status *status)
{
FloatParts64 p;
@@ -3740,6 +3757,11 @@ float64 int16_to_float64_scalbn(int16_t a, int scale,
float_status *status)
return int64_to_float64_scalbn(a, scale, status);
}
+float64 int128_to_float64(int64_t hi, uint64_t lo, float_status *status)
+{
+return int128_to_float64_scalbn(hi, lo, 0, status);
+}
+
float64 int64_to_float64(int64_t a, float_status *status)
{
return int64_to_float64_scalbn(a, 0, status);
@@ -3788,6 +3810,14 @@ bfloat16 int16_to_bfloat16(int16_t a, float_status
*status)
return int64_to_bfloat16_scalbn(a, 0, status);
}
+float128 int128_to_float128(int64_t hi, uint64_t lo, float_status *status)
+{
+FloatParts128 p;
+
+parts_sint_to_float2(, hi, lo, 0, status);
+return float128_round_pack_canonical(, status);
+}
+
float128 int64_to_float128(int64_t a, float_status *status)
{
FloatParts128 p;
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 2ef4fec3d0..5a2165a187 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -166,6 +166,7 @@ float32 uint64_to_float32(uint64_t, float_status *status);
float64 int16_to_float64_scalbn(int16_t, int, float_status *status);
float64 int32_to_float64_scalbn(int32_t, int, float_status *status);
float64 int64_to_float64_scalbn(int64_t, int, float_status *status);
+float64 int128_to_float64_scalbn(int64_t, uint64_t, int, float_status *status);
float64 uint16_to_float64_scalbn(uint16_t, int, float_status *status);
float64 uint32_to_float64_scalbn(uint32_t, int, float_status *status);
float64 uint64_to_float64_scalbn(uint64_t, int, float_status *status);
@@ -175,6 +176,7 @@ float64 uint128_to_float64_scalbn(uint64_t, uint64_t, int,
float64 int16_to_float64(int16_t, float_status *status);
float64 int32_to_float64(int32_t, float_status *status);
float64 int64_to_float64(int64_t, float_status *status);
+float64 int128_to_float64(int64_t, uint64_t, float_status *status);
float64
[RFC PATCH 3/6] softfloat: add float*_to_uint128 conversion methods
From: Matheus Ferst
Implements parts_float_to_uint2 based on parts_float_to_uint logic. The
new methods return the lower part of the result through the "lo"
pointer.
Signed-off-by: Matheus Ferst
---
fpu/softfloat-parts.c.inc | 71 +++
fpu/softfloat.c | 51
include/fpu/softfloat.h | 8 +
3 files changed, 130 insertions(+)
diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
index 5f7f107a0d..2767aeac03 100644
--- a/fpu/softfloat-parts.c.inc
+++ b/fpu/softfloat-parts.c.inc
@@ -1164,6 +1164,77 @@ static uint64_t partsN(float_to_uint)(FloatPartsN *p,
FloatRoundMode rmode,
return r;
}
+static uint64_t partsN(float_to_uint2)(FloatPartsN *p, FloatRoundMode rmode,
+ int scale, float_status *s, uint64_t
*lo)
+{
+int flags = 0;
+uint64_t hi;
+
+switch (p->cls) {
+case float_class_snan:
+flags |= float_flag_invalid_snan;
+/* fall through */
+case float_class_qnan:
+flags |= float_flag_invalid;
+hi = UINT64_MAX;
+*lo = UINT64_MAX;
+break;
+
+case float_class_inf:
+flags = float_flag_invalid | float_flag_invalid_cvti;
+if (p->sign) {
+hi = 0;
+*lo = 0;
+} else {
+hi = UINT64_MAX;
+*lo = UINT64_MAX;
+}
+break;
+
+case float_class_zero:
+*lo = 0;
+return 0;
+
+case float_class_normal:
+if (parts_round_to_int_normal(p, rmode, scale, N - 2)) {
+flags = float_flag_inexact;
+if (p->cls == float_class_zero) {
+hi = 0;
+*lo = 0;
+break;
+}
+}
+
+if (p->sign) {
+flags = float_flag_invalid | float_flag_invalid_cvti;
+hi = 0;
+*lo = 0;
+} else if (p->exp <= DECOMPOSED_BINARY_POINT) {
+hi = 0;
+*lo = p->frac_hi >> (DECOMPOSED_BINARY_POINT - p->exp);
+} else if (p->exp <= 127) {
+int shift = 127 - p->exp;
+hi = shr_double(0, p->frac_hi, shift);
+if (N > 64) {
+*lo = shr_double(p->frac_hi, p->frac_lo, shift);
+} else {
+*lo = shr_double(p->frac_hi, 0, shift);
+}
+} else {
+flags = float_flag_invalid | float_flag_invalid_cvti;
+hi = UINT64_MAX;
+*lo = UINT64_MAX;
+}
+break;
+
+default:
+g_assert_not_reached();
+}
+
+float_raise(flags, s);
+return hi;
+}
+
/*
* Integer to float conversions
*
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index ac3d6f5ab0..fe4320060c 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -850,6 +850,16 @@ static uint64_t parts128_float_to_uint(FloatParts128 *p,
FloatRoundMode rmode,
#define parts_float_to_uint(P, R, Z, M, S) \
PARTS_GENERIC_64_128(float_to_uint, P)(P, R, Z, M, S)
+static uint64_t parts64_float_to_uint2(FloatParts64 *p, FloatRoundMode rmode,
+ int scale, float_status *s,
+ uint64_t *lo);
+static uint64_t parts128_float_to_uint2(FloatParts128 *p, FloatRoundMode rmode,
+int scale, float_status *s,
+uint64_t *lo);
+
+#define parts_float_to_uint2(P, R, Z, S, H) \
+PARTS_GENERIC_64_128(float_to_uint2, P)(P, R, Z, S, H)
+
static void parts64_sint_to_float(FloatParts64 *p, int64_t a,
int scale, float_status *s);
static void parts128_sint_to_float(FloatParts128 *p, int64_t a,
@@ -3451,6 +3461,15 @@ uint64_t float64_to_uint64_scalbn(float64 a,
FloatRoundMode rmode, int scale,
return parts_float_to_uint(, rmode, scale, UINT64_MAX, s);
}
+uint64_t float64_to_uint128_scalbn(float64 a, FloatRoundMode rmode, int scale,
+ float_status *s, uint64_t *lo)
+{
+FloatParts64 p;
+
+float64_unpack_canonical(, a, s);
+return parts_float_to_uint2(, rmode, scale, s, lo);
+}
+
uint16_t bfloat16_to_uint16_scalbn(bfloat16 a, FloatRoundMode rmode,
int scale, float_status *s)
{
@@ -3496,6 +3515,16 @@ static uint64_t float128_to_uint64_scalbn(float128 a,
FloatRoundMode rmode,
return parts_float_to_uint(, rmode, scale, UINT64_MAX, s);
}
+static uint64_t float128_to_uint128_scalbn(float128 a, FloatRoundMode rmode,
+ int scale, float_status *s,
+ uint64_t *lo)
+{
+FloatParts128 p;
+
+float128_unpack_canonical(, a, s);
+return parts_float_to_uint2(, rmode, scale, s, lo);
+}
+
uint8_t float16_to_uint8(float16 a, float_status *s)
{
return float16_to_uint8_scalbn(a, s->float_rounding_mode, 0, s);
@@ -3546,6 +3575,11 @@ uint64_t
Re: [RFC PATCH v7 09/29] target/loongarch: Add TLB instruction support
On 3/28/22 06:57, Xiaojuan Yang wrote:
+static void output_empty(DisasContext *ctx, arg_empty *a,
+ const char *mnemonic)
+{
+}
No, you must still do
output(ctx, mnemonic, "");
+static bool trans_tlbwr(DisasContext *ctx, arg_tlbwr *a)
+{
+if (check_plv(ctx)) {
+return false;
+}
+gen_helper_tlbwr(cpu_env);
+
+if (ctx->mem_idx != MMU_DA_IDX) {
+tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
+ctx->base.is_jmp = DISAS_EXIT;
+}
You may want to create a helper function for this condition.
+static bool trans_tlbclr(DisasContext *ctx, arg_tlbclr *a)
+{
+if (check_plv(ctx)) {
+return false;
+}
+gen_helper_tlbclr(cpu_env);
+tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
+ctx->base.is_jmp = DISAS_EXIT;
Missing it here...
+static bool trans_tlbflush(DisasContext *ctx, arg_tlbflush *a)
+{
+if (check_plv(ctx)) {
+return false;
+}
+gen_helper_tlbflush(cpu_env);
+tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
+ctx->base.is_jmp = DISAS_EXIT;
+return true;
+}
... and here.
r~
Re: [PATCH v6 0/5] optimize the downtime for vfio migration
On Sat, 26 Mar 2022 14:02:21 +0800
"Longpeng(Mike)" wrote:
> From: Longpeng
>
> Hi guys,
>
> In vfio migration resume phase, the cost would increase if the
> vfio device has more unmasked vectors. We try to optimize it in
> this series.
>
> You can see the commit message in PATCH 6 for details.
>
> Patch 1-3 are simple cleanups and fixup.
> Patch 4 are the preparations for the optimization.
> Patch 5 optimizes the vfio msix setup path.
>
> v5: https://lore.kernel.org/all/20211103081657.1945-1-longpe...@huawei.com/T/
>
> Change v5->v6:
> - remove the Patch 4("kvm: irqchip: extract
> kvm_irqchip_add_deferred_msi_route")
> of v5, and use KVMRouteChange API instead. [Paolo, Longpeng]
>
> Changes v4->v5:
> - setup the notifier and irqfd in the same function to makes
>the code neater.[Alex]
>
> Changes v3->v4:
> - fix several typos and grammatical errors [Alex]
> - remove the patches that fix and clean the MSIX common part
>from this series [Alex]
> - Patch 6:
> - use vector->use directly and fill it with -1 on error
> paths [Alex]
> - add comment before enable deferring to commit [Alex]
> - move the code that do_use/release on vector 0 into an
> "else" branch [Alex]
> - introduce vfio_prepare_kvm_msi_virq_batch() that enables
> the 'defer_kvm_irq_routing' flag [Alex]
> - introduce vfio_commit_kvm_msi_virq_batch() that clears the
> 'defer_kvm_irq_routing' flag and does further work [Alex]
>
> Changes v2->v3:
> - fix two errors [Longpeng]
>
> Changes v1->v2:
> - fix several typos and grammatical errors [Alex, Philippe]
> - split fixups and cleanups into separate patches [Alex, Philippe]
> - introduce kvm_irqchip_add_deferred_msi_route to
>minimize code changes[Alex]
> - enable the optimization in msi setup path[Alex]
>
> Longpeng (Mike) (5):
> vfio: simplify the conditional statements in vfio_msi_enable
> vfio: move re-enabling INTX out of the common helper
> vfio: simplify the failure path in vfio_msi_enable
> Revert "vfio: Avoid disabling and enabling vectors repeatedly in VFIO
> migration"
> vfio: defer to commit kvm irq routing when enable msi/msix
>
> hw/vfio/pci.c | 183 +++---
> hw/vfio/pci.h | 2 +
> 2 files changed, 115 insertions(+), 70 deletions(-)
>
Nice to see you found a solution with Paolo's suggestion for
begin/commit batching. Looks ok to me; I'll queue this for after the
v7.0 QEMU release and look for further reviews and comments in the
interim. Thanks,
Alex
Re: [PATCH v3 for-7.1] vfio/common: remove spurious tpm-crb-cmd misalignment warning
On Wed, 23 Mar 2022 21:31:19 +0100
Eric Auger wrote:
> The CRB command buffer currently is a RAM MemoryRegion and given
> its base address alignment, it causes an error report on
> vfio_listener_region_add(). This region could have been a RAM device
> region, easing the detection of such safe situation but this option
> was not well received. So let's add a helper function that uses the
> memory region owner type to detect the situation is safe wrt
> the assignment. Other device types can be checked here if such kind
> of problem occurs again.
>
> Signed-off-by: Eric Auger
> Reviewed-by: Philippe Mathieu-Daudé
>
> ---
Thanks Eric! I'll queue this for after the v7.0 release with Connie's
and Stefan's reviews. Thanks,
Alex
>
> v2 -> v3:
> - Use TPM_IS_CRB()
>
> v1 -> v2:
> - do not check the MR name but rather the owner type
> ---
> hw/vfio/common.c | 27 ++-
> hw/vfio/trace-events | 1 +
> 2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 080046e3f51..55bc116473e 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -40,6 +40,7 @@
> #include "trace.h"
> #include "qapi/error.h"
> #include "migration/migration.h"
> +#include "sysemu/tpm.h"
>
> VFIOGroupList vfio_group_list =
> QLIST_HEAD_INITIALIZER(vfio_group_list);
> @@ -861,6 +862,22 @@ static void
> vfio_unregister_ram_discard_listener(VFIOContainer *container,
> g_free(vrdl);
> }
>
> +static bool vfio_known_safe_misalignment(MemoryRegionSection *section)
> +{
> +MemoryRegion *mr = section->mr;
> +
> +if (!TPM_IS_CRB(mr->owner)) {
> +return false;
> +}
> +
> +/* this is a known safe misaligned region, just trace for debug purpose
> */
> +trace_vfio_known_safe_misalignment(memory_region_name(mr),
> + section->offset_within_address_space,
> + section->offset_within_region,
> + qemu_real_host_page_size);
> +return true;
> +}
> +
> static void vfio_listener_region_add(MemoryListener *listener,
> MemoryRegionSection *section)
> {
> @@ -884,7 +901,15 @@ static void vfio_listener_region_add(MemoryListener
> *listener,
> if (unlikely((section->offset_within_address_space &
>~qemu_real_host_page_mask) !=
> (section->offset_within_region &
> ~qemu_real_host_page_mask))) {
> -error_report("%s received unaligned region", __func__);
> +if (!vfio_known_safe_misalignment(section)) {
> +error_report("%s received unaligned region %s iova=0x%"PRIx64
> + " offset_within_region=0x%"PRIx64
> + " qemu_real_host_page_mask=0x%"PRIxPTR,
> + __func__, memory_region_name(section->mr),
> + section->offset_within_address_space,
> + section->offset_within_region,
> + qemu_real_host_page_mask);
> +}
> return;
> }
>
> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
> index 0ef1b5f4a65..6f38a2e6991 100644
> --- a/hw/vfio/trace-events
> +++ b/hw/vfio/trace-events
> @@ -100,6 +100,7 @@ vfio_listener_region_add_skip(uint64_t start, uint64_t
> end) "SKIPPING region_add
> vfio_spapr_group_attach(int groupfd, int tablefd) "Attached groupfd %d to
> liobn fd %d"
> vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add
> [iommu] 0x%"PRIx64" - 0x%"PRIx64
> vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void
> *vaddr) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p]"
> +vfio_known_safe_misalignment(const char *name, uint64_t iova, uint64_t
> offset_within_region, uint64_t page_size) "Region \"%s\" iova=0x%"PRIx64"
> offset_within_region=0x%"PRIx64" qemu_real_host_page_mask=0x%"PRIxPTR ":
> cannot be mapped for DMA"
> vfio_listener_region_add_no_dma_map(const char *name, uint64_t iova,
> uint64_t size, uint64_t page_size) "Region \"%s\" 0x%"PRIx64"
> size=0x%"PRIx64" is not aligned to 0x%"PRIx64" and cannot be mapped for DMA"
> vfio_listener_region_del_skip(uint64_t start, uint64_t end) "SKIPPING
> region_del 0x%"PRIx64" - 0x%"PRIx64
> vfio_listener_region_del(uint64_t start, uint64_t end) "region_del
> 0x%"PRIx64" - 0x%"PRIx64
Re: [RFC PATCH v7 06/29] target/loongarch: Add MMU support for LoongArch CPU.
On 3/28/22 06:57, Xiaojuan Yang wrote:
+bool loongarch_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+MMUAccessType access_type, int mmu_idx,
+bool probe, uintptr_t retaddr)
+{
+LoongArchCPU *cpu = LOONGARCH_CPU(cs);
+CPULoongArchState *env = >env;
+hwaddr physical;
+int prot;
+int ret = TLBRET_BADADDR;
+
+/* Data access */
+ret = get_physical_address(env, , , address,
+ access_type, mmu_idx);
+
+if (ret == TLBRET_MATCH) {
+tlb_set_page(cs, address & TARGET_PAGE_MASK,
+ physical & TARGET_PAGE_MASK, prot,
+ mmu_idx, TARGET_PAGE_SIZE);
+qemu_log_mask(CPU_LOG_MMU,
+ "%s address=%" VADDR_PRIx " physical " TARGET_FMT_plx
+ " prot %d\n", __func__, address, physical, prot);
+return true;
+} else {
+qemu_log_mask(CPU_LOG_MMU,
+ "%s address=%" VADDR_PRIx " ret %d\n", __func__, address,
+ ret);
+}
+if (probe) {
+return false;
+} else {
Drop the else and unindent.
+raise_mmu_exception(env, address, access_type, ret);
+do_raise_exception(env, cs->exception_index, retaddr);
You do not need do_raise_exception here, as you have already assigned to
cs->exception_index (obviously). Just cpu_loop_exit_restore(cs, retaddr).
Otherwise,
Reviewed-by: Richard Henderson
r~
Re: [RFC PATCH v7 05/29] target/loongarch: Add constant timer support
On 3/28/22 06:57, Xiaojuan Yang wrote:
+void cpu_loongarch_store_constant_timer_config(LoongArchCPU *cpu,
+ uint64_t value)
+{
+CPULoongArchState *env = >env;
+uint64_t now, next;
+
+env->CSR_TCFG = value;
+if (value & CONSTANT_TIMER_ENABLE) {
+now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+next = now + (value & CONSTANT_TIMER_TICK_MASK) * TIMER_PERIOD;
+timer_mod(>timer, next);
+}
If CONSTANT_TIMER_ENABLE is not set, you need to use timer_del() to turn off any existing
timer.
+void loongarch_constant_timer_cb(void *opaque)
+{
+LoongArchCPU *cpu = opaque;
+CPULoongArchState *env = >env;
+uint64_t now, next;
+
+if (FIELD_EX64(env->CSR_TCFG, CSR_TCFG, PERIODIC)) {
+now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+next = now + (env->CSR_TCFG & CONSTANT_TIMER_TICK_MASK) * TIMER_PERIOD;
+timer_mod(>timer, next);
+} else {
+env->CSR_TCFG = FIELD_DP64(env->CSR_TCFG, CSR_TCFG, EN, 0);
+}
+
+env->CSR_ESTAT |= 1 << IRQ_TIMER;
+cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
I think this is wrong and you should be using loongarch_cpu_set_irq (which is misplaced
for you to be able to do so).
@@ -297,4 +302,9 @@ enum {
#define LOONGARCH_CPU_TYPE_NAME(model) model LOONGARCH_CPU_TYPE_SUFFIX
#define CPU_RESOLVING_TYPE TYPE_LOONGARCH_CPU
+void loongarch_constant_timer_cb(void *opaque);
+uint64_t cpu_loongarch_get_constant_timer_counter(LoongArchCPU *cpu);
+uint64_t cpu_loongarch_get_constant_timer_ticks(LoongArchCPU *cpu);
+void cpu_loongarch_store_constant_timer_config(LoongArchCPU *cpu,
+ uint64_t value);
These can go in internals.h.
r~
Re: [RFC PATCH v7 02/29] target/loongarch: Add CSRs definition
On 3/28/22 06:57, Xiaojuan Yang wrote: +++ b/target/loongarch/cpu.h @@ -11,6 +11,7 @@ #include "exec/cpu-defs.h" #include "fpu/softfloat-types.h" #include "hw/registerfields.h" +#include "cpu-csr.h" Do you need this include here? I would hope that there would be only a few extra files that need this. Otherwise, Reviewed-by: Richard Henderson r~
Re: [PATCH v5 00/13] KVM: mm: fd-based approach for supporting KVM guest private memory
On Mon, Mar 28, 2022, Quentin Perret wrote: > Hi Sean, > > Thanks for the reply, this helps a lot. > > On Monday 28 Mar 2022 at 17:13:10 (+), Sean Christopherson wrote: > > On Thu, Mar 24, 2022, Quentin Perret wrote: > > > For Protected KVM (and I suspect most other confidential computing > > > solutions), guests have the ability to share some of their pages back > > > with the host kernel using a dedicated hypercall. This is necessary > > > for e.g. virtio communications, so these shared pages need to be mapped > > > back into the VMM's address space. I'm a bit confused about how that > > > would work with the approach proposed here. What is going to be the > > > approach for TDX? > > > > > > It feels like the most 'natural' thing would be to have a KVM exit > > > reason describing which pages have been shared back by the guest, and to > > > then allow the VMM to mmap those specific pages in response in the > > > memfd. Is this something that has been discussed or considered? > > > > The proposed solution is to exit to userspace with a new exit reason, > > KVM_EXIT_MEMORY_ERROR, > > when the guest makes the hypercall to request conversion[1]. The private > > fd itself > > will never allow mapping memory into userspace, instead userspace will need > > to punch > > a hole in the private fd backing store. The absense of a valid mapping in > > the private > > fd is how KVM detects that a pfn is "shared" (memslots without a private fd > > are always > > shared)[2]. > > Right. I'm still a bit confused about how the VMM is going to get the > shared page mapped in its page-table. Once it has punched a hole into > the private fd, how is it supposed to access the actual physical page > that the guest shared? The guest doesn't share a _host_ physical page, the guest shares a _guest_ physical page. Until host userspace converts the gfn to shared and thus maps the gfn=>hva via mmap(), the guest is blocked and can't read/write/exec the memory. AFAIK, no architecture allows in-place decryption of guest private memory. s390 allows a page to be "made accessible" to the host for the purposes of swap, and other architectures will have similar behavior for migrating a protected VM, but those scenarios are not sharing the page (and they also make the page inaccessible to the guest). > Is there an assumption somewhere that the VMM should have this page mapped in > via an alias that it can legally access only once it has punched a hole at > the corresponding offset in the private fd or something along those lines? Yes, the VMM must have a completely separate VMA. The VMM doesn't haven't to wait until the conversion to mmap() the shared variant, though obviously it will potentially consume double the memory if the VMM actually populates both the private and shared backing stores. > > The key point is that KVM never decides to convert between shared and > > private, it's > > always a userspace decision. Like normal memslots, where userspace has > > full control > > over what gfns are a valid, this gives userspace full control over whether > > a gfn is > > shared or private at any given time. > > I'm understanding this as 'the VMM is allowed to punch holes in the > private fd whenever it wants'. Is this correct? >From the kernel's perspective, yes, the VMM can punch holes at any time. From >a "do I want to DoS my guest" perspective, the VMM must honor its contract with the guest and not spuriously unmap private memory. > What happens if it does so for a page that a guest hasn't shared back? When the hole is punched, KVM will unmap the corresponding private SPTEs. If the guest is still accessing the page as private, the next access will fault and KVM will exit to userspace with KVM_EXIT_MEMORY_ERROR. Of course the guest is probably hosed if the hole punch was truly spurious, as at least hardware-based protected VMs effectively destroy data when a private page is unmapped from the guest private SPTEs. E.g. Linux guests for TDX and SNP will panic/terminate in such a scenario as they will get a fault (injected by trusted hardware/firmware) saying that the guest is trying to access an unaccepted/unvalidated page (TDX and SNP require the guest to explicit accept all private pages that aren't part of the guest's initial pre-boot image). > > Another important detail is that this approach means the kernel and KVM > > treat the > > shared backing store and private backing store as independent, albeit > > related, > > entities. This is very deliberate as it makes it easier to reason about > > what is > > and isn't allowed/required. E.g. the kernel only needs to handle freeing > > private > > memory, there is no special handling for conversion to shared because no > > such path > > exists as far as host pfns are concerned. And userspace doesn't need any > > new "rules" > > for protecting itself against a malicious guest, e.g. userspace already > > needs to > > ensure that it has a
Re: [RFC PATCH v7 08/29] target/loongarch: Add LoongArch IOCSR instruction
On 3/28/22 06:57, Xiaojuan Yang wrote:
+uint64_t helper_iocsr_read(CPULoongArchState *env, target_ulong r_addr,
+ uint32_t size)
+{
+int cpuid = env_cpu(env)->cpu_index;
+CPUState *cs = qemu_get_cpu(cpuid);
+env = cs->env_ptr;
+uint64_t ret;
+
+/*
+ * Adjust the per core address such as 0x10xx(IPI)/0x18xx(EXTIOI)
+ */
+if (((r_addr & 0xff00) == 0x1000) || ((r_addr & 0xff00) == 0x1800)) {
+r_addr = r_addr + ((target_ulong)(cpuid & 0x3) << 8);
+}
+
+switch (size) {
+case 1:
+ret = address_space_ldub(>address_space_iocsr, r_addr,
+ MEMTXATTRS_UNSPECIFIED, NULL);
+break;
+case 2:
+ret = address_space_lduw(>address_space_iocsr, r_addr,
+ MEMTXATTRS_UNSPECIFIED, NULL);
+break;
+case 4:
+ret = address_space_ldl(>address_space_iocsr, r_addr,
+MEMTXATTRS_UNSPECIFIED, NULL);
+break;
+case 8:
+ret = address_space_ldq(>address_space_iocsr, r_addr,
+MEMTXATTRS_UNSPECIFIED, NULL);
+break;
+default:
+break;
+}
+
+return ret;
+}
You should have seen an uninitialized use of 'ret' here.
The default case should be g_assert_not_reached().
And the same in helper_iocsr_write.
r~
Re: [PATCH v2] coreaudio: Notify error in coreaudio_init_out
On 26/2/22 12:59, Akihiko Odaki wrote: Otherwise, the audio subsystem tries to use the voice and eventually aborts due to the maximum number of samples in the buffer is not set. Signed-off-by: Akihiko Odaki Reviewed-by: Christian Schoenebeck --- audio/coreaudio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/audio/coreaudio.c b/audio/coreaudio.c index d8a21d3e507..d7cfdcc4fc4 100644 --- a/audio/coreaudio.c +++ b/audio/coreaudio.c @@ -604,6 +604,8 @@ static int coreaudio_init_out(HWVoiceOut *hw, struct audsettings *as, coreaudio_playback_logerr(status, "Could not remove voice property change listener\n"); } + +return -1; } return 0; Merged as commit bd7819de22.
Re: [RFC PATCH v7 07/29] target/loongarch: Add LoongArch CSR instruction
On 3/28/22 06:57, Xiaojuan Yang wrote:
+#define CSR_OFF(X) \
+ [LOONGARCH_CSR_##X] = offsetof(CPULoongArchState, CSR_##X)
+#define CSR_OFF_ARRAY(X, N) \
+ [LOONGARCH_CSR_##X(N)] = offsetof(CPULoongArchState, CSR_##X[N])
+
+static const int csr_offsets[] = {
You cannot put a variable data definition into a header file like this.
It has put this data structure into every object file.
This belongs in csr_helper.c, probably.
You should add
[LOONGARCH_CSR_CPUID] = offsetof(CPUState, cpu_index) - offsetof(ArchCPU,
env),
rather than special-casing this in helper_csr_rdq.
+static inline int cpu_csr_offset(unsigned csr_num)
+{
+if (csr_num < ARRAY_SIZE(csr_offsets)) {
+return csr_offsets[csr_num];
+}
+return 0;
+}
This does not need to be inline, and could easily live in csr_helper.c.
+target_ulong helper_csr_rdq(CPULoongArchState *env, uint64_t csr)
+{
+LoongArchCPU *cpu;
+int64_t v;
+
+switch (csr) {
+case LOONGARCH_CSR_PGD:
+if (env->CSR_TLBRERA & 0x1) {
+v = env->CSR_TLBRBADV;
+} else {
+v = env->CSR_BADV;
+}
+
+if ((v >> 63) & 0x1) {
+v = env->CSR_PGDH;
+} else {
+v = env->CSR_PGDL;
+}
+break;
+case LOONGARCH_CSR_CPUID:
+v = (env_cpu(env))->cpu_index;
+break;
+case LOONGARCH_CSR_TVAL:
+cpu = LOONGARCH_CPU(env_cpu(env));
+v = cpu_loongarch_get_constant_timer_ticks(cpu);
+break;
+default:
+break;
+}
+
+return v;
+}
You should have seen a compiler warning for 'v' uninitialized here, via the
default path.
The default path should not be reachable, because of code in trans_csrrd, and so could be
written as g_assert_not_reachable(). However, I strongly suggest you split this function
so that you do not need a switch here at all. With CPUID now handled via cpu_csr_offset,
there are only two helpers needed.
+target_ulong helper_csr_wrq(CPULoongArchState *env, target_ulong val,
+uint64_t csr)
+{
+LoongArchCPU *cpu;
+int64_t old_v = -1;
+
+switch (csr) {
+case LOONGARCH_CSR_ESTAT:
+/* Only IS[1:0] can be write */
"can be written", and then again below.
+env->CSR_ESTAT = FIELD_DP64(env->CSR_ESTAT, CSR_ESTAT, IS, val & 0x3);
+break;
+case LOONGARCH_CSR_ASID:
+old_v = env->CSR_ASID;
+/* Only ASID filed of CSR_ASID can be write. */
+env->CSR_ASID = FIELD_DP64(env->CSR_ASID, CSR_ASID, ASID,
+ val & R_CSR_ASID_ASID_MASK);
+if (old_v != val) {
+tlb_flush(env_cpu(env));
+}
+break;
+case LOONGARCH_CSR_TCFG:
+cpu = LOONGARCH_CPU(env_cpu(env));
+old_v = env->CSR_TCFG;
+cpu_loongarch_store_constant_timer_config(cpu, val);
+break;
+case LOONGARCH_CSR_TICLR:
+old_v = 0;
+env->CSR_ESTAT &= ~(1 << IRQ_TIMER);
+cpu_reset_interrupt(env_cpu(env), CPU_INTERRUPT_HARD);
Surely the TIMER irq is not the only interrupt.
The placement of the reset looks incorrect.
And again, I suggest that you *not* use a switch, but use separate helper
functions.
+target_ulong helper_csr_xchgq(CPULoongArchState *env, target_ulong new_val,
+ target_ulong mask, uint64_t csr_num)
+{
+unsigned csr_offset = cpu_csr_offset(csr_num);
+if (csr_offset == 0) {
+/* Undefined CSR: read as 0, writes are ignored. */
+return 0;
+}
+
+uint64_t *csr = (void *)env + csr_offset;
+uint64_t old_val = *csr;
+
+new_val = (new_val & mask) | (old_val & ~mask);
+
+if (csr_num == LOONGARCH_CSR_TCFG) {
+LoongArchCPU *cpu = LOONGARCH_CPU(env_cpu(env));
+cpu_loongarch_store_constant_timer_config(cpu, new_val);
+} else {
+*csr = new_val;
You're only handling one of the special cases from helper_csr_wrq, so I cannot believe
this is correct.
I think you should not have a helper_csr_xchgq function, but reuse the read/write
infrastructure from the other csr access instructions. Note this would also fix...
+static bool trans_csrwr(DisasContext *ctx, arg_csrwr *a)
+{
+TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE);
+TCGv src1 = gpr_src(ctx, a->rd, EXT_NONE);
+
+if (check_plv(ctx) || ro_csr(a->csr)) {
+return false;
+}
+
+unsigned csr_offset = cpu_csr_offset(a->csr);
+if (csr_offset == 0) {
+/* CSR is undefined: write ignored. */
+return true;
+}
+
+if ((a->csr == LOONGARCH_CSR_ASID) || (a->csr == LOONGARCH_CSR_TCFG) ||
+(a->csr == LOONGARCH_CSR_TICLR) || (a->csr == LOONGARCH_CSR_ESTAT)) {
+gen_helper_csr_wrq(dest, cpu_env, src1, tcg_constant_i64(a->csr));
ASID change may result in page translation changes (which is why you did tlb_flush). This
also means that the page you are now executing could change translation, so you have
Re: [RFC PATCH v7 00/29] Add LoongArch softmmu support
On 3/28/22 06:57, Xiaojuan Yang wrote: This series patch add softmmu support for LoongArch. The latest kernel: * https://github.com/loongson/linux/tree/loongarch-next The latest uefi: * https://github.com/loongson/edk2 * https://github.com/loongson/edk2-platforms The manual: * https://github.com/loongson/LoongArch-Documentation/releases/tag/2021.10.11 You can get LoongArch qemu series like this: git clone https://github.com/loongson/qemu.git git checkout tcg-dev I strongly suggest that you rebase *without* the patches for linux-user/loongarch64/. If you do not do this, you will be stalled until your linux kernel patches are merged. Testing this rebase locally, there is only a minor patch conflict in target/loongarch: Add LoongArch interrupt and exception handle which suggests that the EXCP_* names be removed from target/loongarch: Add core definition The EXCCODE_* names be introduced early instead, and all of the uses updated. r~
Re: [PATCH] target/arm: Fix MTE access checks for disabled SEL2
On 3/28/22 11:31, Idan Horowitz wrote: While not mentioned anywhere in the actual specification text, the HCR_EL2.ATA bit is treated as '1' when EL2 is disabled at the current security state. This can be observed in the psuedo-code implementation of AArch64.AllocationTagAccessIsEnabled(). Signed-off-by: Idan Horowitz --- target/arm/helper.c| 2 +- target/arm/internals.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) I was immediately thinking, didn't I just fix this? But I was patching pauth. Anyway, good catch. Reviewed-by: Richard Henderson r~
Re: [PATCH v5 00/13] KVM: mm: fd-based approach for supporting KVM guest private memory
Hi Sean, Thanks for the reply, this helps a lot. On Monday 28 Mar 2022 at 17:13:10 (+), Sean Christopherson wrote: > On Thu, Mar 24, 2022, Quentin Perret wrote: > > For Protected KVM (and I suspect most other confidential computing > > solutions), guests have the ability to share some of their pages back > > with the host kernel using a dedicated hypercall. This is necessary > > for e.g. virtio communications, so these shared pages need to be mapped > > back into the VMM's address space. I'm a bit confused about how that > > would work with the approach proposed here. What is going to be the > > approach for TDX? > > > > It feels like the most 'natural' thing would be to have a KVM exit > > reason describing which pages have been shared back by the guest, and to > > then allow the VMM to mmap those specific pages in response in the > > memfd. Is this something that has been discussed or considered? > > The proposed solution is to exit to userspace with a new exit reason, > KVM_EXIT_MEMORY_ERROR, > when the guest makes the hypercall to request conversion[1]. The private fd > itself > will never allow mapping memory into userspace, instead userspace will need > to punch > a hole in the private fd backing store. The absense of a valid mapping in > the private > fd is how KVM detects that a pfn is "shared" (memslots without a private fd > are always > shared)[2]. Right. I'm still a bit confused about how the VMM is going to get the shared page mapped in its page-table. Once it has punched a hole into the private fd, how is it supposed to access the actual physical page that the guest shared? Is there an assumption somewhere that the VMM should have this page mapped in via an alias that it can legally access only once it has punched a hole at the corresponding offset in the private fd or something along those lines? > The key point is that KVM never decides to convert between shared and > private, it's > always a userspace decision. Like normal memslots, where userspace has full > control > over what gfns are a valid, this gives userspace full control over whether a > gfn is > shared or private at any given time. I'm understanding this as 'the VMM is allowed to punch holes in the private fd whenever it wants'. Is this correct? What happens if it does so for a page that a guest hasn't shared back? > Another important detail is that this approach means the kernel and KVM treat > the > shared backing store and private backing store as independent, albeit related, > entities. This is very deliberate as it makes it easier to reason about what > is > and isn't allowed/required. E.g. the kernel only needs to handle freeing > private > memory, there is no special handling for conversion to shared because no such > path > exists as far as host pfns are concerned. And userspace doesn't need any new > "rules" > for protecting itself against a malicious guest, e.g. userspace already needs > to > ensure that it has a valid mapping prior to accessing guest memory (or be > able to > handle any resulting signals). A malicious guest can DoS itself by > instructing > userspace to communicate over memory that is currently mapped private, but > there > are no new novel attack vectors from the host's perspective as coercing the > host > into accessing an invalid mapping after shared=>private conversion is just a > variant > of a use-after-free. Interesting. I was (maybe incorrectly) assuming that it would be difficult to handle illegal host accesses w/ TDX. IOW, this would essentially crash the host. Is this remotely correct or did I get that wrong? > One potential conversions that's TBD (at least, I think it is, I haven't read > through > this most recent version) is how to support populating guest private memory > with > non-zero data, e.g. to allow in-place conversion of the initial guest > firmware instead > of having to an extra memcpy(). Right. FWIW, in the pKVM case we should be pretty immune to this I think. The initial firmware is loaded in guest memory by the hypervisor itself (the EL2 code in arm64 speak) as the first vCPU starts running. And that firmware can then use e.g. virtio to load the guest payload and measure/check it. IOW, we currently don't have expectations regarding the initial state of guest memory, but it might be handy to have support for pre-loading the payload in the future (should save a copy as you said). > [1] KVM will also exit to userspace with the same info on "implicit" > conversions, > i.e. if the guest accesses the "wrong" GPA. Neither SEV-SNP nor TDX > mandate > explicit conversions in their guest<->host ABIs, so KVM has to support > implicit > conversions :-/ > > [2] Ideally (IMO), KVM would require userspace to completely remove the > private memslot, > but that's too slow due to use of SRCU in both KVM and userspace (QEMU at > least uses > SRCU for memslot changes).
Re: [PATCH 3/3] qga/commands-posix: Fix listing ifaces for Solaris
On Mon, 21 Mar 2022 10:14:57 +0100
Michal Prívozník wrote:
> On 3/20/22 22:38, Andrew Deason wrote:
> > The code for guest-network-get-interfaces needs a couple of small
> > adjustments for Solaris:
> >
> > - The results from SIOCGIFHWADDR are documented as being in ifr_addr,
> > not ifr_hwaddr (ifr_hwaddr doesn't exist on Solaris).
> >
> > - The implementation of guest_get_network_stats is Linux-specific, so
> > hide it under #ifdef CONFIG_LINUX. On non-Linux, we just won't
> > provide network interface stats.
> >
> > Signed-off-by: Andrew Deason
> > ---
> > qga/commands-posix.c | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index bd0d67f674..c0b00fc488 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -2781,20 +2781,21 @@ guest_find_interface(GuestNetworkInterfaceList
> > *head,
> > return head->value;
> > }
> > }
> >
> > return NULL;
> > }
> >
> > static int guest_get_network_stats(const char *name,
> > GuestNetworkInterfaceStat *stats)
> > {
> > +#ifdef CONFIG_LINUX
> > int name_len;
> > char const *devinfo = "/proc/net/dev";
> > FILE *fp;
> > char *line = NULL, *colon;
> > size_t n = 0;
> > fp = fopen(devinfo, "r");
> > if (!fp) {
> > return -1;
> > }
> > name_len = strlen(name);
> > @@ -2836,20 +2837,21 @@ static int guest_get_network_stats(const char *name,
> > stats->tx_errs = tx_errs;
> > stats->tx_dropped = tx_dropped;
> > fclose(fp);
> > g_free(line);
> > return 0;
> > }
> > }
> > fclose(fp);
> > g_free(line);
> > g_debug("/proc/net/dev: Interface '%s' not found", name);
> > +#endif /* CONFIG_LINUX */
>
> I wonder whether we should signal this somehow. I mean, have something
> like this:
>
> #else /* !CONFIG_LINUX */
> g_debug("Stats reporting available only for Linux");
> #endif /* !CONFIG_LINUX */
>
> > return -1;
> > }
>
> A counter argument is that if fopen() above fails then -1 is returned
> without any error/debug message reported. And stats fetching is best
> effort anyway.
Ping for this stack. Should I just go ahead and add the above? Sorry if
I was expected to respond to this; I don't disagree but I also saw the
existing silent-failure code path so it doesn't seem like it matters.
I could add debug messages for both silent-failure code paths, maybe as
a separate commit afterwards?
--
Andrew Deason
adea...@sinenomine.net
[PATCH] target/arm: Fix MTE access checks for disabled SEL2
While not mentioned anywhere in the actual specification text, the
HCR_EL2.ATA bit is treated as '1' when EL2 is disabled at the current
security state. This can be observed in the psuedo-code implementation
of AArch64.AllocationTagAccessIsEnabled().
Signed-off-by: Idan Horowitz
---
target/arm/helper.c| 2 +-
target/arm/internals.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 16c2628f8f..7d14650615 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7176,7 +7176,7 @@ static CPAccessResult access_mte(CPUARMState *env, const
ARMCPRegInfo *ri,
{
int el = arm_current_el(env);
-if (el < 2 && arm_feature(env, ARM_FEATURE_EL2)) {
+if (el < 2 && arm_is_el2_enabled(env)) {
uint64_t hcr = arm_hcr_el2_eff(env);
if (!(hcr & HCR_ATA) && (!(hcr & HCR_E2H) || !(hcr & HCR_TGE))) {
return CP_ACCESS_TRAP_EL2;
diff --git a/target/arm/internals.h b/target/arm/internals.h
index a34be2e459..7f696cd36a 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1094,7 +1094,7 @@ static inline bool
allocation_tag_access_enabled(CPUARMState *env, int el,
&& !(env->cp15.scr_el3 & SCR_ATA)) {
return false;
}
-if (el < 2 && arm_feature(env, ARM_FEATURE_EL2)) {
+if (el < 2 && arm_is_el2_enabled(env)) {
uint64_t hcr = arm_hcr_el2_eff(env);
if (!(hcr & HCR_ATA) && (!(hcr & HCR_E2H) || !(hcr & HCR_TGE))) {
return false;
--
2.35.1
Re: [PATCH 02/15] tests/docker: remove test targets
On 3/28/22 18:44, Alex Bennée wrote:
Paolo Bonzini writes:
Signed-off-by: Paolo Bonzini
---
tests/docker/Makefile.include | 18 --
1 file changed, 18 deletions(-)
diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index a6a5a20949..8248cfdb4f 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -99,24 +99,6 @@ docker-binfmt-image-debian-%:
$(DOCKER_FILES_DIR)/debian-bootstrap.docker
{ echo "You will need to build $(EXECUTABLE)"; exit
1;},\
"CHECK", "debian-$* exists"))
-# These are test targets
-USER_TCG_TARGETS=$(patsubst %-linux-user,qemu-%,$(filter
%-linux-user,$(TARGET_DIRS)))
-EXEC_COPY_TESTS=$(patsubst %,docker-exec-copy-test-%, $(USER_TCG_TARGETS))
-
-$(EXEC_COPY_TESTS): docker-exec-copy-test-%:
$(DOCKER_FILES_DIR)/empty.docker
Should probably clean-up the empty.docker while you are at it. It's a
niche command but I wonder how we would copy new tests now?
You can always document them as an alternative to this patch. :)
Paolo
Re: [PATCH 01/15] tests/docker: remove dead code
On 3/28/22 18:18, Alex Bennée wrote:
debian-powerpc-user-cross was the only linux-user powered cross builder
and it was removed in commit 80394ccf21 ("tests/docker: remove
debian-powerpc-user-cross", 2019-09-26). Remove all the infrastructure
around it since it is now unused.
It doesn't remove it all - we still have the
docker-binfmt-image-debian-% rule (which I'd like to keep). Anyway:
Reviewed-by: Alex Bennée
Yeah, I wasn't sure about that one.
Paolo
Re: [PATCH 00/15] tests/docker and tests/tcg cleanup and diet
On 3/28/22 17:53, Richard Henderson wrote: This is also a first step towards moving the cross-compilation infrastructure from tests/tcg to all of QEMU, so that it can be used to build firmware binaries. Yay! However, the tricore special cases broke: Silly pasto: diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh index f195d2d873..c0022b47a6 100755 --- a/tests/tcg/configure.sh +++ b/tests/tcg/configure.sh @@ -334,7 +334,7 @@ for target in $target_list; do echo "BUILD_STATIC=y" >> $config_target_mak echo "CC=\$(DOCKER_SCRIPT) cc --cc $container_cross_cc -i qemu/$container_image -s $source_path --" >> $config_target_mak if test -n "$container_cross_as"; then - echo "AS=\$(DOCKER_SCRIPT) cc --cc $container_cross_cc -i qemu/$container_image -s $source_path --" >> $config_target_mak + echo "AS=\$(DOCKER_SCRIPT) cc --cc $container_cross_as -i qemu/$container_image -s $source_path --" >> $config_target_mak fi if test -n "$container_cross_ld"; then echo "LD=\$(DOCKER_SCRIPT) cc --cc $container_cross_ld -i qemu/$container_image -s $source_path --" >> $config_target_mak Paolo
[RFC PATCH v3 2/4] target/riscv: smstateen check for h/senvcfg
Accesses to henvcfg, henvcfgh and senvcfg are allowed
only if corresponding bit in mstateen0/hstateen0 is
enabled. Otherwise an illegal instruction trap is
generated.
Signed-off-by: Mayuresh Chitale
---
target/riscv/csr.c | 82 ++
1 file changed, 76 insertions(+), 6 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e3dafc37ef..dda254a6c9 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -37,6 +37,35 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
}
/* Predicates */
+static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
+{
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
+bool virt = riscv_cpu_virt_enabled(env);
+
+if (!cpu->cfg.ext_smstateen) {
+return RISCV_EXCP_NONE;
+}
+
+if (!(env->mstateen[0] & (1UL << bit))) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+if (virt) {
+if (!(env->hstateen[0] & (1UL << bit))) {
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+}
+}
+
+if (mode == PRV_U) {
+if (!(env->sstateen[0] & (1UL << bit))) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+}
+
+return RISCV_EXCP_NONE;
+}
+
static RISCVException fs(CPURISCVState *env, int csrno)
{
#if !defined(CONFIG_USER_ONLY)
@@ -1476,6 +1505,13 @@ static RISCVException write_menvcfgh(CPURISCVState *env,
int csrno,
static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
target_ulong *val)
{
+RISCVException ret;
+
+ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
*val = env->senvcfg;
return RISCV_EXCP_NONE;
}
@@ -1484,15 +1520,27 @@ static RISCVException write_senvcfg(CPURISCVState *env,
int csrno,
target_ulong val)
{
uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | SENVCFG_CBZE;
+RISCVException ret;
-env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
+ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
return RISCV_EXCP_NONE;
}
static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
target_ulong *val)
{
+RISCVException ret;
+
+ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
*val = env->henvcfg;
return RISCV_EXCP_NONE;
}
@@ -1501,6 +1549,12 @@ static RISCVException write_henvcfg(CPURISCVState *env,
int csrno,
target_ulong val)
{
uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | HENVCFG_CBZE;
+RISCVException ret;
+
+ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
if (riscv_cpu_mxl(env) == MXL_RV64) {
mask |= HENVCFG_PBMTE | HENVCFG_STCE;
@@ -1514,6 +1568,13 @@ static RISCVException write_henvcfg(CPURISCVState *env,
int csrno,
static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
target_ulong *val)
{
+RISCVException ret;
+
+ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
*val = env->henvcfg >> 32;
return RISCV_EXCP_NONE;
}
@@ -1523,9 +1584,14 @@ static RISCVException write_henvcfgh(CPURISCVState *env,
int csrno,
{
uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
uint64_t valh = (uint64_t)val << 32;
+RISCVException ret;
-env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
+ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
return RISCV_EXCP_NONE;
}
@@ -1547,7 +1613,8 @@ static RISCVException write_mstateen(CPURISCVState *env,
int csrno,
target_ulong new_val)
{
uint64_t *reg;
-uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+ (1UL << SMSTATEEN0_HSENVCFG);
reg = >mstateen[csrno - CSR_MSTATEEN0];
write_smstateen(env, reg, wr_mask, new_val);
@@ -1568,7 +1635,8 @@ static RISCVException write_mstateenh(CPURISCVState *env,
int csrno,
{
uint64_t *reg;
uint64_t val;
-uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
+uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
+ (1UL << SMSTATEEN0_HSENVCFG);
reg = >mstateen[csrno - CSR_MSTATEEN0H];
val = (uint64_t)new_val << 32;
@@ -1590,7 +1658,8 @@ static RISCVException write_hstateen(CPURISCVState *env,
int csrno,
target_ulong
[RFC PATCH v3 4/4] target/riscv: smstateen check for AIA/IMSIC
If smstateen is implemented then accesses to AIA
registers CSRS, IMSIC CSRs and other IMSIC registers
is controlled by setting of corresponding bits in
mstateen/hstateen registers. Otherwise an illegal
instruction trap or virtual instruction trap is
generated.
Signed-off-by: Mayuresh Chitale
---
target/riscv/csr.c | 248 -
1 file changed, 246 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 658f51bd40..93866fd773 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -66,6 +66,53 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env,
int mode, int bit)
return RISCV_EXCP_NONE;
}
+static RISCVException smstateen_aia_acc_ok(CPURISCVState *env, int csrno)
+{
+int bit, mode;
+
+switch (csrno) {
+case CSR_SSETEIPNUM:
+case CSR_SCLREIPNUM:
+case CSR_SSETEIENUM:
+case CSR_SCLREIENUM:
+case CSR_STOPEI:
+case CSR_VSSETEIPNUM:
+case CSR_VSCLREIPNUM:
+case CSR_VSSETEIENUM:
+case CSR_VSCLREIENUM:
+case CSR_VSTOPEI:
+case CSR_HSTATUS:
+mode = PRV_S;
+bit = SMSTATEEN0_IMSIC;
+break;
+
+case CSR_SIEH:
+case CSR_SIPH:
+case CSR_HVIPH:
+case CSR_HVICTL:
+case CSR_HVIPRIO1:
+case CSR_HVIPRIO2:
+case CSR_HVIPRIO1H:
+case CSR_HVIPRIO2H:
+case CSR_VSIEH:
+case CSR_VSIPH:
+mode = PRV_S;
+bit = SMSTATEEN0_AIA;
+break;
+
+case CSR_SISELECT:
+case CSR_VSISELECT:
+mode = PRV_S;
+bit = SMSTATEEN0_SVSLCT;
+break;
+
+default:
+return RISCV_EXCP_NONE;
+}
+
+return smstateen_acc_ok(env, mode, bit);
+}
+
static RISCVException fs(CPURISCVState *env, int csrno)
{
#if !defined(CONFIG_USER_ONLY)
@@ -1047,6 +1094,13 @@ static int rmw_xiselect(CPURISCVState *env, int csrno,
target_ulong *val,
target_ulong new_val, target_ulong wr_mask)
{
target_ulong *iselect;
+RISCVException ret;
+
+/* Check if smstateen is enabled and this access is allowed */
+ret = smstateen_aia_acc_ok(env, csrno);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
/* Translate CSR number for VS-mode */
csrno = aia_xlate_vs_csrno(env, csrno);
@@ -1129,7 +1183,9 @@ static int rmw_xireg(CPURISCVState *env, int csrno,
target_ulong *val,
bool virt;
uint8_t *iprio;
int ret = -EINVAL;
-target_ulong priv, isel, vgein;
+target_ulong priv, isel, vgein = 0;
+CPUState *cs = env_cpu(env);
+RISCVCPU *cpu = RISCV_CPU(cs);
/* Translate CSR number for VS-mode */
csrno = aia_xlate_vs_csrno(env, csrno);
@@ -1158,11 +1214,20 @@ static int rmw_xireg(CPURISCVState *env, int csrno,
target_ulong *val,
};
/* Find the selected guest interrupt file */
-vgein = (virt) ? get_field(env->hstatus, HSTATUS_VGEIN) : 0;
+if (virt) {
+if (!cpu->cfg.ext_smstateen ||
+(env->hstateen[0] & (1UL << SMSTATEEN0_IMSIC))) {
+vgein = get_field(env->hstatus, HSTATUS_VGEIN);
+}
+}
if (ISELECT_IPRIO0 <= isel && isel <= ISELECT_IPRIO15) {
/* Local interrupt priority registers not available for VS-mode */
if (!virt) {
+if (priv == PRV_S && cpu->cfg.ext_smstateen &&
+!(env->hstateen[0] & (1UL << SMSTATEEN0_AIA))) {
+goto done;
+}
ret = rmw_iprio(riscv_cpu_mxl_bits(env),
isel, iprio, val, new_val, wr_mask,
(priv == PRV_M) ? IRQ_M_EXT : IRQ_S_EXT);
@@ -1196,6 +1261,13 @@ static int rmw_xsetclreinum(CPURISCVState *env, int
csrno, target_ulong *val,
int ret = -EINVAL;
bool set, pend, virt;
target_ulong priv, isel, vgein, xlen, nval, wmask;
+RISCVException excp;
+
+/* Check if smstateen is enabled and this access is allowed */
+excp = smstateen_aia_acc_ok(env, csrno);
+if (excp != RISCV_EXCP_NONE) {
+return excp;
+}
/* Translate CSR number for VS-mode */
csrno = aia_xlate_vs_csrno(env, csrno);
@@ -1314,6 +1386,13 @@ static int rmw_xtopei(CPURISCVState *env, int csrno,
target_ulong *val,
bool virt;
int ret = -EINVAL;
target_ulong priv, vgein;
+RISCVException excp;
+
+/* Check if smstateen is enabled and this access is allowed */
+excp = smstateen_aia_acc_ok(env, csrno);
+if (excp != RISCV_EXCP_NONE) {
+return excp;
+}
/* Translate CSR number for VS-mode */
csrno = aia_xlate_vs_csrno(env, csrno);
@@ -1625,6 +1704,12 @@ static RISCVException write_mstateen(CPURISCVState *env,
int csrno,
wr_mask |= 1UL << SMSTATEEN0_FCSR;
}
+if (riscv_feature(env, RISCV_FEATURE_AIA)) {
+wr_mask |= (1UL << SMSTATEEN0_IMSIC) |
+ (1UL << SMSTATEEN0_AIA) |
+ (1UL << SMSTATEEN0_SVSLCT);
+}
+
write_smstateen(env, reg, wr_mask, new_val);
[RFC PATCH v3 3/4] target/riscv: smstateen check for fcsr
If smstateen is implemented and sstateen0.fcsr is clear
then the floating point operations must return illegal
instruction exception.
Signed-off-by: Mayuresh Chitale
---
target/riscv/csr.c | 24
1 file changed, 24 insertions(+)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index dda254a6c9..658f51bd40 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -73,6 +73,10 @@ static RISCVException fs(CPURISCVState *env, int csrno)
!RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
return RISCV_EXCP_ILLEGAL_INST;
}
+
+if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
+return smstateen_acc_ok(env, PRV_U, SMSTATEEN0_FCSR);
+}
#endif
return RISCV_EXCP_NONE;
}
@@ -1617,6 +1621,10 @@ static RISCVException write_mstateen(CPURISCVState *env,
int csrno,
(1UL << SMSTATEEN0_HSENVCFG);
reg = >mstateen[csrno - CSR_MSTATEEN0];
+if (riscv_has_ext(env, RVF)) {
+wr_mask |= 1UL << SMSTATEEN0_FCSR;
+}
+
write_smstateen(env, reg, wr_mask, new_val);
return RISCV_EXCP_NONE;
@@ -1641,6 +1649,10 @@ static RISCVException write_mstateenh(CPURISCVState
*env, int csrno,
reg = >mstateen[csrno - CSR_MSTATEEN0H];
val = (uint64_t)new_val << 32;
val |= *reg & 0x;
+if (riscv_has_ext(env, RVF)) {
+wr_mask |= 1UL << SMSTATEEN0_FCSR;
+}
+
write_smstateen(env, reg, wr_mask, val);
return RISCV_EXCP_NONE;
@@ -1662,6 +1674,10 @@ static RISCVException write_hstateen(CPURISCVState *env,
int csrno,
(1UL << SMSTATEEN0_HSENVCFG);
int index = csrno - CSR_HSTATEEN0;
+if (riscv_has_ext(env, RVF)) {
+wr_mask |= 1UL << SMSTATEEN0_FCSR;
+}
+
reg = >hstateen[index];
wr_mask &= env->mstateen[index];
write_smstateen(env, reg, wr_mask, new_val);
@@ -1686,6 +1702,10 @@ static RISCVException write_hstateenh(CPURISCVState
*env, int csrno,
uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
(1UL << SMSTATEEN0_HSENVCFG);
+if (riscv_has_ext(env, RVF)) {
+wr_mask |= 1UL << SMSTATEEN0_FCSR;
+}
+
reg = >hstateen[index];
val = (uint64_t)new_val << 32;
val |= *reg & 0x;
@@ -1711,6 +1731,10 @@ static RISCVException write_sstateen(CPURISCVState *env,
int csrno,
int index = csrno - CSR_SSTATEEN0;
bool virt = riscv_cpu_virt_enabled(env);
+if (riscv_has_ext(env, RVF)) {
+wr_mask |= 1UL << SMSTATEEN0_FCSR;
+}
+
reg = >sstateen[index];
if (virt) {
wr_mask &= env->mstateen[index];
--
2.17.1
