Re: [PATCH 2/3] vdpa: load vlan configuration at NIC startup
On Wed, Sep 14, 2022 at 11:44 PM Si-Wei Liu wrote:
>
>
>
> On 9/14/2022 2:57 PM, Eugenio Perez Martin wrote:
> > On Wed, Sep 14, 2022 at 1:33 PM Si-Wei Liu wrote:
> >>
> >>
> >> On 9/14/2022 3:20 AM, Jason Wang wrote:
> >>> On Fri, Sep 9, 2022 at 4:02 PM Eugenio Perez Martin
> >>> wrote:
> On Fri, Sep 9, 2022 at 8:40 AM Jason Wang wrote:
> > On Fri, Sep 9, 2022 at 2:38 PM Jason Wang wrote:
> >> On Wed, Sep 7, 2022 at 12:36 AM Eugenio Pérez
> >> wrote:
> >>> To have enabled vlans at device startup may happen in the destination
> >>> of
> >>> a live migration, so this configuration must be restored.
> >>>
> >>> At this moment the code is not accessible, since SVQ refuses to start
> >>> if
> >>> vlan feature is exposed by the device.
> >>>
> >>> Signed-off-by: Eugenio Pérez
> >>> ---
> >>>net/vhost-vdpa.c | 46
> >>> --
> >>>1 file changed, 44 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> >>> index 4bc3fd01a8..ecbfd08eb9 100644
> >>> --- a/net/vhost-vdpa.c
> >>> +++ b/net/vhost-vdpa.c
> >>> @@ -100,6 +100,8 @@ static const uint64_t vdpa_svq_device_features =
> >>>BIT_ULL(VIRTIO_NET_F_RSC_EXT) |
> >>>BIT_ULL(VIRTIO_NET_F_STANDBY);
> >>>
> >>> +#define MAX_VLAN(1 << 12) /* Per 802.1Q definition */
> >>> +
> >>>VHostNetState *vhost_vdpa_get_vhost_net(NetClientState *nc)
> >>>{
> >>>VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> >>> @@ -423,6 +425,47 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState
> >>> *s,
> >>>return *s->status != VIRTIO_NET_OK;
> >>>}
> >>>
> >>> +static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
> >>> + const VirtIONet *n,
> >>> + uint16_t vid)
> >>> +{
> >>> +ssize_t dev_written = vhost_vdpa_net_load_cmd(s,
> >>> VIRTIO_NET_CTRL_VLAN,
> >>> +
> >>> VIRTIO_NET_CTRL_VLAN_ADD,
> >>> + , sizeof(vid));
> >>> +if (unlikely(dev_written < 0)) {
> >>> +return dev_written;
> >>> +}
> >>> +
> >>> +if (unlikely(*s->status != VIRTIO_NET_OK)) {
> >>> +return -EINVAL;
> >>> +}
> >>> +
> >>> +return 0;
> >>> +}
> >>> +
> >>> +static int vhost_vdpa_net_load_vlan(VhostVDPAState *s,
> >>> +const VirtIONet *n)
> >>> +{
> >>> +uint64_t features = n->parent_obj.guest_features;
> >>> +
> >>> +if (!(features & BIT_ULL(VIRTIO_NET_F_CTRL_VLAN))) {
> >>> +return 0;
> >>> +}
> >>> +
> >>> +for (int i = 0; i < MAX_VLAN >> 5; i++) {
> >>> +for (int j = 0; n->vlans[i] && j <= 0x1f; j++) {
> >>> +if (n->vlans[i] & (1U << j)) {
> >>> +int r = vhost_vdpa_net_load_single_vlan(s, n, (i <<
> >>> 5) + j);
> >> This seems to cause a lot of latency if the driver has a lot of vlans.
> >>
> >> I wonder if it's simply to let all vlan traffic go by disabling
> >> CTRL_VLAN feature at vDPA layer.
> The guest will not be able to recover that vlan configuration.
> >>> Spec said it's best effort and we actually don't do this for
> >>> vhost-net/user for years and manage to survive.
> >> I thought there's a border line between best effort and do nothing. ;-)
> >>
> > I also think it is worth at least trying to migrate it for sure. For
> > MAC there may be too many entries, but VLAN should be totally
> > manageable (and the information is already there, the bitmap is
> > actually being migrated).
> The vlan bitmap is migrated, though you'd still need to add vlan filter
> one by one through ctrl vq commands, correct? AFAIK you can add one
> filter for one single vlan ID at a time via VIRTIO_NET_CTRL_VLAN_ADD.
>
> >
> >> I think that the reason this could survive is really software
> >> implementation specific - say if the backend doesn't start with VLAN
> >> filtering disabled (meaning allow all VLAN traffic to pass) then it
> >> would become a problem. This won't be a problem for almost PF NICs but
> >> may be problematic for vDPA e.g. VF backed VDPAs.
> > I'd say the driver would expect all vlan filters to be cleared after a
> > reset, isn't it?
> If I understand the intended behavior (from QEMU implementation)
> correctly, when VIRTIO_NET_F_CTRL_VLAN is negotiated it would start with
> all VLANs filtered (meaning only untagged traffic can be received and
> traffic with VLAN tag would get dropped). However, if
> VIRTIO_NET_F_CTRL_VLAN is not negotiated, all traffic including untagged
> and tagged can be received.
>
> > The spec doesn't
Re: [PATCH 2/3] vdpa: load vlan configuration at NIC startup
On Wed, Sep 14, 2022 at 7:33 PM Si-Wei Liu wrote:
>
>
>
> On 9/14/2022 3:20 AM, Jason Wang wrote:
> > On Fri, Sep 9, 2022 at 4:02 PM Eugenio Perez Martin
> > wrote:
> >> On Fri, Sep 9, 2022 at 8:40 AM Jason Wang wrote:
> >>> On Fri, Sep 9, 2022 at 2:38 PM Jason Wang wrote:
> On Wed, Sep 7, 2022 at 12:36 AM Eugenio Pérez
> wrote:
> > To have enabled vlans at device startup may happen in the destination of
> > a live migration, so this configuration must be restored.
> >
> > At this moment the code is not accessible, since SVQ refuses to start if
> > vlan feature is exposed by the device.
> >
> > Signed-off-by: Eugenio Pérez
> > ---
> > net/vhost-vdpa.c | 46 --
> > 1 file changed, 44 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > index 4bc3fd01a8..ecbfd08eb9 100644
> > --- a/net/vhost-vdpa.c
> > +++ b/net/vhost-vdpa.c
> > @@ -100,6 +100,8 @@ static const uint64_t vdpa_svq_device_features =
> > BIT_ULL(VIRTIO_NET_F_RSC_EXT) |
> > BIT_ULL(VIRTIO_NET_F_STANDBY);
> >
> > +#define MAX_VLAN(1 << 12) /* Per 802.1Q definition */
> > +
> > VHostNetState *vhost_vdpa_get_vhost_net(NetClientState *nc)
> > {
> > VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> > @@ -423,6 +425,47 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState
> > *s,
> > return *s->status != VIRTIO_NET_OK;
> > }
> >
> > +static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
> > + const VirtIONet *n,
> > + uint16_t vid)
> > +{
> > +ssize_t dev_written = vhost_vdpa_net_load_cmd(s,
> > VIRTIO_NET_CTRL_VLAN,
> > +
> > VIRTIO_NET_CTRL_VLAN_ADD,
> > + , sizeof(vid));
> > +if (unlikely(dev_written < 0)) {
> > +return dev_written;
> > +}
> > +
> > +if (unlikely(*s->status != VIRTIO_NET_OK)) {
> > +return -EINVAL;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static int vhost_vdpa_net_load_vlan(VhostVDPAState *s,
> > +const VirtIONet *n)
> > +{
> > +uint64_t features = n->parent_obj.guest_features;
> > +
> > +if (!(features & BIT_ULL(VIRTIO_NET_F_CTRL_VLAN))) {
> > +return 0;
> > +}
> > +
> > +for (int i = 0; i < MAX_VLAN >> 5; i++) {
> > +for (int j = 0; n->vlans[i] && j <= 0x1f; j++) {
> > +if (n->vlans[i] & (1U << j)) {
> > +int r = vhost_vdpa_net_load_single_vlan(s, n, (i << 5)
> > + j);
> This seems to cause a lot of latency if the driver has a lot of vlans.
>
> I wonder if it's simply to let all vlan traffic go by disabling
> CTRL_VLAN feature at vDPA layer.
> >> The guest will not be able to recover that vlan configuration.
> > Spec said it's best effort and we actually don't do this for
> > vhost-net/user for years and manage to survive.
> I thought there's a border line between best effort and do nothing. ;-)
>
> I think that the reason this could survive is really software
> implementation specific - say if the backend doesn't start with VLAN
> filtering disabled (meaning allow all VLAN traffic to pass) then it
> would become a problem. This won't be a problem for almost PF NICs but
> may be problematic for vDPA e.g. VF backed VDPAs.
So it looks like an issue of the implementation. If CTRL_VLAN is not
negotiated, the device should disable vlan filters.
> >
> >>> Another idea is to extend the spec to allow us to accept a bitmap of
> >>> the vlan ids via a single command, then we will be fine.
> >>>
> >> I'm not sure if adding more ways to configure something is the answer,
> >> but I'd be ok to implement it.
> >>
> >> Another idea is to allow the sending of many CVQ commands in parallel.
> >> It shouldn't be very hard to achieve using exposed buffers as ring
> >> buffers, and it will short down the start of the devices with many
> >> features.
> > In the extreme case, what if guests decide to filter all of the vlans?
> > It means we need MAX_VLAN commands which may exceeds the size of the
> > control virtqueue.
> Indeed, that's a case where a single flat device state blob would be
> more efficient for hardware drivers to apply than individual control
> command messages do.
Right, so we can optimize the spec for this.
Thanks
>
> -Siwei
> >
> > Thanks
> >
> >> Thanks!
> >>
> >>> Thanks
> >>>
> Thanks
>
> > +if (unlikely(r != 0)) {
> > +return r;
> > +}
> > +}
> > +}
> > +}
> > +
> > +
Re: [PATCH v2 3/6] vhost-net: vhost-user: update vhost_net_virtqueue_reset()
On Wed, Sep 14, 2022 at 2:21 PM Xuan Zhuo wrote:
>
> On Wed, 14 Sep 2022 11:13:29 +0800, Jason Wang wrote:
> >
> > 在 2022/9/12 11:10, Kangjie Xu 写道:
> > > Update vhost_net_virtqueue_reset() for vhost-user scenario.
> > >
> > > In order to reuse some functions, we process the idx for
> > > vhost-user scenario because vhost_get_vq_index behave
> > > differently for vhost-user.
> > >
> > > Signed-off-by: Kangjie Xu
> > > Signed-off-by: Xuan Zhuo
> > > ---
> > > hw/net/vhost_net.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > index ea896ea75b..25e5665489 100644
> > > --- a/hw/net/vhost_net.c
> > > +++ b/hw/net/vhost_net.c
> > > @@ -545,6 +545,9 @@ void vhost_net_virtqueue_reset(VirtIODevice *vdev,
> > > NetClientState *nc,
> > > assert(vhost_ops);
> > >
> > > idx = vhost_ops->vhost_get_vq_index(>dev, vq_index);
> > > +if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) {
> > > +idx -= net->dev.vq_index;
> > > +}
> >
> >
> > This looks tricky. Any reason we can't simply use vq_index for both
> > vhost-user and vhost-net?
>
>
> static int vhost_user_get_vq_index(struct vhost_dev *dev, int idx)
> {
> assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
>
> return idx;
> }
>
>
> static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
> {
> assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
>
> return idx - dev->vq_index;
> }
>
> The implementation of these two callbacks is different. The structure of the
> two
> scenarios is different. We may need to do some optimizations in the future.
Yes, but what I meant is, you do
idx -= net->dev.vq_index;
and then
net->dev.vq_index + idx
This is a hint that we should have a better organization of the code.
Thanks
>
> Thanks.
>
>
> >
> > Thanks
> >
> >
> > >
> > > if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
> > > file.index = idx;
> >
>
Re:Re: [PATCH] bugfix:migrate with block-dirty-bitmap (disk size is big enough) can't be finished
Hi ,Vladimir sometimes ,post-copy mode is not the best choice. For instance, Supposing migrate process will take ten minutes,but network may be interruptted In this process . If it does happenthe , memory data of VM will be splitted into two parts, and will not be rollback.This is a bad situation so migrate-start-postcopy will not be setted in conservative scenario. In this case, the migration with block dirty bitmap may not be finished. I think migration of block dirty bitmap should not dependent on post-copy or pre-copy mode. Best regards At 2022-09-10 18:18:04, "Vladimir Sementsov-Ogievskiy" wrote: >On 9/10/22 09:35, liuhaiwei wrote: >> From: liuhaiwei >> >> bug description as https://gitlab.com/qemu-project/qemu/-/issues/1203 >> Usually,we use the precopy or postcopy mode to migrate block dirty bitmap. >> but if block-dirty-bitmap size more than threshold size,we cannot entry the >> migration_completion in migration_iteration_run function >> To solve this problem, we can setting the pending size to a fake >> value(threshold-1 or 0) to tell migration_iteration_run function to entry >> the migration_completion,if pending size > threshold size >> > > >Actually, bitmaps migrate in postcopy. So, you should start postcopy for it to >work (qmp command migrate-start-postcopy). This command simply set the boolean >variable, so that in migration_iteration_run() we'll move to postcopy when >needed. So, you can start this command immediately after migrate command, or >even before it, but after setting the "dirty-bitmaps" capability. > >Fake pending is a wrong thing to do, it means that you will make downtime to >be larger than expected. > >-- >Best regards, >Vladimir
Re: [PATCH 1/2] target/i386: fix cmpxchg with 32-bit register destination
On 9/12/22 09:55, Richard Henderson wrote:
* Compute...
Why bother passing NULL, and fixing it up at each use...
+static void gen_op_mov_reg_v(DisasContext *s, MemOp ot, int reg, TCGv
t0)
+{
+ gen_op_deposit_reg_v(s, ot, reg, NULL, t0);
}
... when you can just as easily pass in the register here?
Because dest can be fixed up to either cpu_regs[reg - 4] for high-byte
registers, or cpu_regs[reg] for everything else.
We have an outstanding bug report that suggests that the move to eax must use
the deposit in both cases:
https://gitlab.com/qemu-project/qemu/-/issues/508
Ok, so that's two bugs. But both of them can indeed be fixed with
gen_op_deposit_reg_v.
Paolo
Re: [PATCH 27/37] target/i386: Use tcg gvec ops for pmovmskb
On Tue, Sep 13, 2022 at 10:17 AM Richard Henderson
wrote:
>
> On 9/12/22 00:04, Paolo Bonzini wrote:
> > +while (vec_len > 8) {
> > +vec_len -= 8;
> > +tcg_gen_shli_tl(s->T0, s->T0, 8);
> > +tcg_gen_ld8u_tl(t, cpu_env, offsetof(CPUX86State,
> > xmm_t0.ZMM_B(vec_len - 1)));
> > +tcg_gen_or_tl(s->T0, s->T0, t);
> > }
>
> The shl + or is deposit, for those hosts that have it,
> and will be re-expanded to shl + or for those that don't:
>
> tcg_gen_ld8u_tl(t, ...);
> tcg_gen_deposit_tl(s->T0, t, s->T0, 8, TARGET_LONG_BITS - 8);
What you get from that is an shl(t, 56) followed by extract2 (i.e.
SHRD). Yeah there are targets with a native deposit (x86 itself could
add PDEP/PEXT support I guess) but I find it hard to believe that it
outperforms a simple shl + or.
If we want to get clever, I should instead load ZMM_B(vec_len - 1)
directly into the *high* byte of t, using ZMM_L or ZMM_Q, and then
issue the extract2 myself.
Paolo
Re: [PATCH 30/37] target/i386: reimplement 0x0f 0x10-0x17, add AVX
On Tue, Sep 13, 2022 at 12:14 PM Richard Henderson
wrote:
> > +static void gen_VMOVLPx(DisasContext *s, CPUX86State *env, X86DecodedInsn
> > *decode)
> > +{
> > +int vec_len = sse_vec_len(s, decode);
> > +
> > +tcg_gen_ld_i64(s->tmp1_i64, cpu_env, decode->op[2].offset +
> > offsetof(XMMReg, XMM_Q(0)));
> > +tcg_gen_gvec_mov(MO_64, decode->op[0].offset, decode->op[1].offset,
> > vec_len, vec_len);
> > +tcg_gen_st_i64(s->tmp1_i64, cpu_env, decode->op[0].offset +
> > offsetof(XMMReg, XMM_Q(0)));
> > +}
>
> You've just been moving i64 pieces in the other functions, why is this one
> different using
> a gvec move in the middle? I do wonder if a generic helper moving
> offset->offset, with
> the comparison wouldn't be helpful within these functions, even when you know
> off1 !=
> off2, due to Q(0) vs Q(1).
Because this one is the only one that has a VEX.256 version (the
operand is type "x"
rather than "dq" as in MOVHLPS, MOVLHPS, MOVHPx).
Paolo
Paolo
Re: [PATCH v2 04/30] tests/avocado: add explicit timeout for s390 TCG tests
On 14/09/2022 16.59, Alex Bennée wrote:
We don't want to rely on the soon to be reduced default time. These
tests are still slow for something we want to run in CI though.
Signed-off-by: Alex Bennée
---
tests/avocado/boot_linux.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
index 67a24fe51c..4f07c27ac6 100644
--- a/tests/avocado/boot_linux.py
+++ b/tests/avocado/boot_linux.py
@@ -130,6 +130,8 @@ class BootLinuxS390X(LinuxTest):
:avocado: tags=arch:s390x
"""
+timeout = 240
+
@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
def test_s390_ccw_virtio_tcg(self):
"""
Reviewed-by: Thomas Huth
Re: [PATCH] vfio/common: Do not g_free in vfio_get_iommu_info
On Wed, Sep 14, 2022 at 02:03:25PM -0600, Alex Williamson wrote:
> > > > +container->pgsizes = 4096;
> > >
> > > This might be a separate question/issue: I wonder if we should use
> > > "sysconf(_SC_PAGE_SIZE)" here instead of 4096.
> > >
> > > With a kernel using a larger page size, e.g. CONFIG_ARM64_64K_PAGES,
> > > the IO page size is likely to be 64K too. If the ioctl fails, this
> > > default 4K setup won't work.
> >
> > Perhaps, but IIRC this solution came about because we originally forgot
> > to expose the IOMMU_INFO flag to indicate the pgsize field was valid.
> > At the time we only supported 4K systems, so it made sense to provide
> > this default, though it is indeed dated.
> >
> > TBH, I don't really see why we should try to continue if the ioctl
> > itself fails, so maybe this should be:
OK. Makes sense to me.
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index ace9562a9ba1..ad188b7649e6 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> > @@ -2111,29 +2111,31 @@ static int vfio_connect_container(VFIOGroup *group,
> > AddressSpace *as,
> > {
> > struct vfio_iommu_type1_info *info;
> >
> > -/*
> > - * FIXME: This assumes that a Type1 IOMMU can map any 64-bit
> > - * IOVA whatsoever. That's not actually true, but the current
> > - * kernel interface doesn't tell us what it can map, and the
> > - * existing Type1 IOMMUs generally support any IOVA we're
> > - * going to actually try in practice.
> > - */
> > ret = vfio_get_iommu_info(container, );
> > +if (ret) {
>
> Clearly untested,
>
>ret = -errno;
>
> > +error_setg_errno(errp, -ret, "Failed to get VFIO IOMMU info");
> > +goto enable_discards_exit:;
There is a ":" in-between :)
Re: [PATCH] vfio/common: Do not g_free in vfio_get_iommu_info
On Wed, 14 Sep 2022 13:53:39 -0600
Alex Williamson wrote:
> On Wed, 14 Sep 2022 12:02:56 -0700
> Nicolin Chen wrote:
>
> > Hi Alex,
> >
> > On Wed, Sep 14, 2022 at 12:10:29PM -0600, Alex Williamson wrote:
> >
> > > > > > Its caller vfio_connect_container() assigns a default value
> > > > > > to info->iova_pgsizes, even if vfio_get_iommu_info() fails.
> > > > > > This would result in a "Segmentation fault" error, when the
> > > > > > VFIO_IOMMU_GET_INFO ioctl errors out.
> > > > > >
> > > > > > Since the caller has g_free already, drop the g_free in its
> > > > > > rollback routine and add a line of comments to highlight it.
> > > > >
> > > > > There's basically two ways to fix this:
> > > > >
> > > > > - return *info in any case, even on error
> > > > > - free *info on error, and make sure that the caller doesn't try to
> > > > > access *info if the function returned !0
> > > > >
> > > > > The problem with the first option is that the caller will access
> > > > > invalid
> > > > > information if it neglects to check the return code, and that might
> > > > > lead
> > > > > to not-that-obvious errors; in the second case, a broken caller would
> > > > > at
> > > > > least fail quickly with a segfault. The current code is easier to fix
> > > > > with the first option.
> > > > >
> > > > > I think I'd prefer the second option; but obviously maintainer's
> > > > > choice.
> > > >
> > > > The caller does check rc all the time. So I made a smaller fix
> > > > (the first option). Attaching the git-diff for the second one.
> > > >
> > > > Alex, please let me know which one you prefer. Thanks!
> >
> > > I think we can do better than that, I don't think we need to maintain
> > > the existing grouping, and that FIXME comment is outdated and has
> > > drifted from the relevant line of code. What about:
> >
> > Your patch looks good and clean to me (some nits inline).
> >
> > Would you please send and merge it, replacing mine?
> >
> > > + /*
> > > + * Setup defaults for container pgsizes and dma_max_mappings if
> > > not
> > > + * provided by kernel below.
> > > */
> >
> > Indentation is misaligned at the first line.
>
> Oops, will run through checkpatch.
>
> >
> > > +container->pgsizes = 4096;
> >
> > This might be a separate question/issue: I wonder if we should use
> > "sysconf(_SC_PAGE_SIZE)" here instead of 4096.
> >
> > With a kernel using a larger page size, e.g. CONFIG_ARM64_64K_PAGES,
> > the IO page size is likely to be 64K too. If the ioctl fails, this
> > default 4K setup won't work.
>
> Perhaps, but IIRC this solution came about because we originally forgot
> to expose the IOMMU_INFO flag to indicate the pgsize field was valid.
> At the time we only supported 4K systems, so it made sense to provide
> this default, though it is indeed dated.
>
> TBH, I don't really see why we should try to continue if the ioctl
> itself fails, so maybe this should be:
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index ace9562a9ba1..ad188b7649e6 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -2111,29 +2111,31 @@ static int vfio_connect_container(VFIOGroup *group,
> AddressSpace *as,
> {
> struct vfio_iommu_type1_info *info;
>
> -/*
> - * FIXME: This assumes that a Type1 IOMMU can map any 64-bit
> - * IOVA whatsoever. That's not actually true, but the current
> - * kernel interface doesn't tell us what it can map, and the
> - * existing Type1 IOMMUs generally support any IOVA we're
> - * going to actually try in practice.
> - */
> ret = vfio_get_iommu_info(container, );
> +if (ret) {
Clearly untested,
ret = -errno;
> +error_setg_errno(errp, -ret, "Failed to get VFIO IOMMU info");
> +goto enable_discards_exit:;
> +}
>
> -if (ret || !(info->flags & VFIO_IOMMU_INFO_PGSIZES)) {
> -/* Assume 4k IOVA page size */
> -info->iova_pgsizes = 4096;
> +if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
> +container->pgsizes = info->iova_pgsizes;
> +} else {
> +container->pgsizes = qemu_real_host_page_size();
> }
> -vfio_host_win_add(container, 0, (hwaddr)-1, info->iova_pgsizes);
> -container->pgsizes = info->iova_pgsizes;
>
> -/* The default in the kernel ("dma_entry_limit") is 65535. */
> -container->dma_max_mappings = 65535;
> -if (!ret) {
> -vfio_get_info_dma_avail(info, >dma_max_mappings);
> -vfio_get_iommu_info_migration(container, info);
> +if (!vfio_get_info_dma_avail(info, >dma_max_mappings)) {
> +container->dma_max_mappings = 65535;
> }
> +vfio_get_iommu_info_migration(container, info);
> g_free(info);
> +
> +/*
> + * FIXME: We should parse
Re: [PATCH] vfio/common: Do not g_free in vfio_get_iommu_info
On Wed, 14 Sep 2022 12:02:56 -0700
Nicolin Chen wrote:
> Hi Alex,
>
> On Wed, Sep 14, 2022 at 12:10:29PM -0600, Alex Williamson wrote:
>
> > > > > Its caller vfio_connect_container() assigns a default value
> > > > > to info->iova_pgsizes, even if vfio_get_iommu_info() fails.
> > > > > This would result in a "Segmentation fault" error, when the
> > > > > VFIO_IOMMU_GET_INFO ioctl errors out.
> > > > >
> > > > > Since the caller has g_free already, drop the g_free in its
> > > > > rollback routine and add a line of comments to highlight it.
> > > >
> > > > There's basically two ways to fix this:
> > > >
> > > > - return *info in any case, even on error
> > > > - free *info on error, and make sure that the caller doesn't try to
> > > > access *info if the function returned !0
> > > >
> > > > The problem with the first option is that the caller will access invalid
> > > > information if it neglects to check the return code, and that might lead
> > > > to not-that-obvious errors; in the second case, a broken caller would at
> > > > least fail quickly with a segfault. The current code is easier to fix
> > > > with the first option.
> > > >
> > > > I think I'd prefer the second option; but obviously maintainer's
> > > > choice.
> > >
> > > The caller does check rc all the time. So I made a smaller fix
> > > (the first option). Attaching the git-diff for the second one.
> > >
> > > Alex, please let me know which one you prefer. Thanks!
>
> > I think we can do better than that, I don't think we need to maintain
> > the existing grouping, and that FIXME comment is outdated and has
> > drifted from the relevant line of code. What about:
>
> Your patch looks good and clean to me (some nits inline).
>
> Would you please send and merge it, replacing mine?
>
> > + /*
> > + * Setup defaults for container pgsizes and dma_max_mappings if not
> > + * provided by kernel below.
> > */
>
> Indentation is misaligned at the first line.
Oops, will run through checkpatch.
>
> > +container->pgsizes = 4096;
>
> This might be a separate question/issue: I wonder if we should use
> "sysconf(_SC_PAGE_SIZE)" here instead of 4096.
>
> With a kernel using a larger page size, e.g. CONFIG_ARM64_64K_PAGES,
> the IO page size is likely to be 64K too. If the ioctl fails, this
> default 4K setup won't work.
Perhaps, but IIRC this solution came about because we originally forgot
to expose the IOMMU_INFO flag to indicate the pgsize field was valid.
At the time we only supported 4K systems, so it made sense to provide
this default, though it is indeed dated.
TBH, I don't really see why we should try to continue if the ioctl
itself fails, so maybe this should be:
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index ace9562a9ba1..ad188b7649e6 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -2111,29 +2111,31 @@ static int vfio_connect_container(VFIOGroup *group,
AddressSpace *as,
{
struct vfio_iommu_type1_info *info;
-/*
- * FIXME: This assumes that a Type1 IOMMU can map any 64-bit
- * IOVA whatsoever. That's not actually true, but the current
- * kernel interface doesn't tell us what it can map, and the
- * existing Type1 IOMMUs generally support any IOVA we're
- * going to actually try in practice.
- */
ret = vfio_get_iommu_info(container, );
+if (ret) {
+error_setg_errno(errp, -ret, "Failed to get VFIO IOMMU info");
+goto enable_discards_exit:;
+}
-if (ret || !(info->flags & VFIO_IOMMU_INFO_PGSIZES)) {
-/* Assume 4k IOVA page size */
-info->iova_pgsizes = 4096;
+if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
+container->pgsizes = info->iova_pgsizes;
+} else {
+container->pgsizes = qemu_real_host_page_size();
}
-vfio_host_win_add(container, 0, (hwaddr)-1, info->iova_pgsizes);
-container->pgsizes = info->iova_pgsizes;
-/* The default in the kernel ("dma_entry_limit") is 65535. */
-container->dma_max_mappings = 65535;
-if (!ret) {
-vfio_get_info_dma_avail(info, >dma_max_mappings);
-vfio_get_iommu_info_migration(container, info);
+if (!vfio_get_info_dma_avail(info, >dma_max_mappings)) {
+container->dma_max_mappings = 65535;
}
+vfio_get_iommu_info_migration(container, info);
g_free(info);
+
+/*
+ * FIXME: We should parse VFIO_IOMMU_TYPE1_INFO_CAP_IOVA_RANGE
+ * information to get the actual window extent rather than assume
+ * a 64-bit IOVA address space.
+ */
+vfio_host_win_add(container, 0, (hwaddr)-1, container->pgsizes);
+
break;
}
case VFIO_SPAPR_TCE_v2_IOMMU:
Re: [PATCH] vfio/common: Do not g_free in vfio_get_iommu_info
Hi Alex, On Wed, Sep 14, 2022 at 12:10:29PM -0600, Alex Williamson wrote: > > > > Its caller vfio_connect_container() assigns a default value > > > > to info->iova_pgsizes, even if vfio_get_iommu_info() fails. > > > > This would result in a "Segmentation fault" error, when the > > > > VFIO_IOMMU_GET_INFO ioctl errors out. > > > > > > > > Since the caller has g_free already, drop the g_free in its > > > > rollback routine and add a line of comments to highlight it. > > > > > > There's basically two ways to fix this: > > > > > > - return *info in any case, even on error > > > - free *info on error, and make sure that the caller doesn't try to > > > access *info if the function returned !0 > > > > > > The problem with the first option is that the caller will access invalid > > > information if it neglects to check the return code, and that might lead > > > to not-that-obvious errors; in the second case, a broken caller would at > > > least fail quickly with a segfault. The current code is easier to fix > > > with the first option. > > > > > > I think I'd prefer the second option; but obviously maintainer's choice. > > > > The caller does check rc all the time. So I made a smaller fix > > (the first option). Attaching the git-diff for the second one. > > > > Alex, please let me know which one you prefer. Thanks! > I think we can do better than that, I don't think we need to maintain > the existing grouping, and that FIXME comment is outdated and has > drifted from the relevant line of code. What about: Your patch looks good and clean to me (some nits inline). Would you please send and merge it, replacing mine? > + /* > + * Setup defaults for container pgsizes and dma_max_mappings if not > + * provided by kernel below. > */ Indentation is misaligned at the first line. > +container->pgsizes = 4096; This might be a separate question/issue: I wonder if we should use "sysconf(_SC_PAGE_SIZE)" here instead of 4096. With a kernel using a larger page size, e.g. CONFIG_ARM64_64K_PAGES, the IO page size is likely to be 64K too. If the ioctl fails, this default 4K setup won't work. Thanks! Nic
Re: [PATCH v8 11/14] qemu-sockets: move and rename SocketAddress_to_str()
* Laurent Vivier (lviv...@redhat.com) wrote:
> Rename SocketAddress_to_str() to socket_uri() and move it to
> util/qemu-sockets.c close to socket_parse().
>
> socket_uri() generates a string from a SocketAddress while
> socket_parse() generates a SocketAddress from a string.
>
> Signed-off-by: Laurent Vivier
Reviewed-by: Dr. David Alan Gilbert
> ---
> include/qemu/sockets.h | 2 +-
> monitor/hmp-cmds.c | 23 +--
> util/qemu-sockets.c| 20
> 3 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index 47194b9732f8..e5a06d2e3729 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -40,6 +40,7 @@ NetworkAddressFamily inet_netfamily(int family);
> int unix_listen(const char *path, Error **errp);
> int unix_connect(const char *path, Error **errp);
>
> +char *socket_uri(SocketAddress *addr);
> SocketAddress *socket_parse(const char *str, Error **errp);
> int socket_connect(SocketAddress *addr, Error **errp);
> int socket_listen(SocketAddress *addr, int num, Error **errp);
> @@ -123,5 +124,4 @@ SocketAddress *socket_address_flatten(SocketAddressLegacy
> *addr);
> * Return 0 on success.
> */
> int socket_address_parse_named_fd(SocketAddress *addr, Error **errp);
> -
> #endif /* QEMU_SOCKETS_H */
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index c6cd6f91dde6..cb35059c2d45 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -197,27 +197,6 @@ void hmp_info_mice(Monitor *mon, const QDict *qdict)
> qapi_free_MouseInfoList(mice_list);
> }
>
> -static char *SocketAddress_to_str(SocketAddress *addr)
> -{
> -switch (addr->type) {
> -case SOCKET_ADDRESS_TYPE_INET:
> -return g_strdup_printf("tcp:%s:%s",
> - addr->u.inet.host,
> - addr->u.inet.port);
> -case SOCKET_ADDRESS_TYPE_UNIX:
> -return g_strdup_printf("unix:%s",
> - addr->u.q_unix.path);
> -case SOCKET_ADDRESS_TYPE_FD:
> -return g_strdup_printf("fd:%s", addr->u.fd.str);
> -case SOCKET_ADDRESS_TYPE_VSOCK:
> -return g_strdup_printf("tcp:%s:%s",
> - addr->u.vsock.cid,
> - addr->u.vsock.port);
> -default:
> -return g_strdup("unknown address type");
> -}
> -}
> -
> void hmp_info_migrate(Monitor *mon, const QDict *qdict)
> {
> MigrationInfo *info;
> @@ -380,7 +359,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
> monitor_printf(mon, "socket address: [\n");
>
> for (addr = info->socket_address; addr; addr = addr->next) {
> -char *s = SocketAddress_to_str(addr->value);
> +char *s = socket_uri(addr->value);
> monitor_printf(mon, "\t%s\n", s);
> g_free(s);
> }
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 83f4bd6fd211..9f6f655fd526 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1077,6 +1077,26 @@ int unix_connect(const char *path, Error **errp)
> return sock;
> }
>
> +char *socket_uri(SocketAddress *addr)
> +{
> +switch (addr->type) {
> +case SOCKET_ADDRESS_TYPE_INET:
> +return g_strdup_printf("tcp:%s:%s",
> + addr->u.inet.host,
> + addr->u.inet.port);
> +case SOCKET_ADDRESS_TYPE_UNIX:
> +return g_strdup_printf("unix:%s",
> + addr->u.q_unix.path);
> +case SOCKET_ADDRESS_TYPE_FD:
> +return g_strdup_printf("fd:%s", addr->u.fd.str);
> +case SOCKET_ADDRESS_TYPE_VSOCK:
> +return g_strdup_printf("tcp:%s:%s",
> + addr->u.vsock.cid,
> + addr->u.vsock.port);
> +default:
> +return g_strdup("unknown address type");
> +}
> +}
>
> SocketAddress *socket_parse(const char *str, Error **errp)
> {
> --
> 2.37.3
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v8 12/14] qemu-sockets: update socket_uri() and socket_parse() to be consistent
* Laurent Vivier (lviv...@redhat.com) wrote:
> To be consistent with socket_uri(), add 'tcp:' prefix for inet type in
> socket_parse(), by default socket_parse() use tcp when no prefix is
> provided (format is host:port).
>
> In socket_uri(), use 'vsock:' prefix for vsock type rather than 'tcp:'
> because it makes a vsock address look like an inet address with CID
> misinterpreted as host.
> Goes back to commit 9aca82ba31 "migration: Create socket-address parameter"
>
> Signed-off-by: Laurent Vivier
Yeh I think that's safer than the last version for migrate URIs
Reviewed-by: Dr. David Alan Gilbert
> ---
> util/qemu-sockets.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 9f6f655fd526..a9926af714c4 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1090,7 +1090,7 @@ char *socket_uri(SocketAddress *addr)
> case SOCKET_ADDRESS_TYPE_FD:
> return g_strdup_printf("fd:%s", addr->u.fd.str);
> case SOCKET_ADDRESS_TYPE_VSOCK:
> -return g_strdup_printf("tcp:%s:%s",
> +return g_strdup_printf("vsock:%s:%s",
> addr->u.vsock.cid,
> addr->u.vsock.port);
> default:
> @@ -1124,6 +1124,11 @@ SocketAddress *socket_parse(const char *str, Error
> **errp)
> if (vsock_parse(>u.vsock, str + strlen("vsock:"), errp)) {
> goto fail;
> }
> +} else if (strstart(str, "tcp:", NULL)) {
> +addr->type = SOCKET_ADDRESS_TYPE_INET;
> +if (inet_parse(>u.inet, str + strlen("tcp:"), errp)) {
> +goto fail;
> +}
> } else {
> addr->type = SOCKET_ADDRESS_TYPE_INET;
> if (inet_parse(>u.inet, str, errp)) {
> --
> 2.37.3
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[PATCH v2] tcg/ppc: Optimize 26-bit jumps
PowerPC64 processors handle direct branches better than indirect
ones, resulting in less stalled cycles and branch misses.
However, PPC's tb_target_set_jmp_target() was only using direct
branches for 16-bit jumps, while PowerPC64's unconditional branch
instructions are able to handle displacements of up to 26 bits.
To take advantage of this, now jumps whose displacements fit in
between 17 and 26 bits are also converted to direct branches.
Signed-off-by: Leandro Lupori
---
v2: use stq to replace all instructions atomically
tcg/ppc/tcg-target.c.inc | 84 +++-
1 file changed, 66 insertions(+), 18 deletions(-)
diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
index 1cbd047ab3..484014cb1a 100644
--- a/tcg/ppc/tcg-target.c.inc
+++ b/tcg/ppc/tcg-target.c.inc
@@ -1847,38 +1847,86 @@ static void tcg_out_mb(TCGContext *s, TCGArg a0)
tcg_out32(s, insn);
}
+#if TCG_TARGET_REG_BITS == 64
+
+static inline uint64_t make_pair(tcg_insn_unit i1, tcg_insn_unit i2)
+{
+#if HOST_BIG_ENDIAN
+return (uint64_t)i1 << 32 | i2;
+#else
+return (uint64_t)i2 << 32 | i1;
+#endif
+}
+
+static inline void ppc64_stq(uintptr_t dst, uint64_t src0, uint64_t src1)
+{
+asm volatile (
+"mr %%r6, %0\n\t"
+"mr %%r7, %1\n\t"
+"stq %%r6, 0(%2)"
+: : "r" (src0), "r" (src1), "r" (dst) : "r6", "r7", "memory");
+}
+
+#endif
+
+static inline void ppc64_replace_insns(uintptr_t rx, uintptr_t rw,
+tcg_insn_unit *insn)
+{
+#if TCG_TARGET_REG_BITS == 64
+uint64_t pair[2];
+
+pair[0] = make_pair(insn[0], insn[1]);
+if (have_isa_2_07) {
+pair[1] = make_pair(insn[2], insn[3]);
+#if HOST_BIG_ENDIAN
+ppc64_stq(rw, pair[0], pair[1]);
+#else
+ppc64_stq(rw, pair[1], pair[0]);
+#endif
+flush_idcache_range(rx, rw, 16);
+} else {
+qatomic_set((uint64_t *)rw, pair[0]);
+flush_idcache_range(rx, rw, 8);
+}
+#endif
+}
+
void tb_target_set_jmp_target(uintptr_t tc_ptr, uintptr_t jmp_rx,
uintptr_t jmp_rw, uintptr_t addr)
{
if (TCG_TARGET_REG_BITS == 64) {
-tcg_insn_unit i1, i2;
+tcg_insn_unit i[4];
intptr_t tb_diff = addr - tc_ptr;
intptr_t br_diff = addr - (jmp_rx + 4);
-uint64_t pair;
-/* This does not exercise the range of the branch, but we do
- still need to be able to load the new value of TCG_REG_TB.
- But this does still happen quite often. */
+/*
+ * This does not exercise the range of the branch, but we do
+ * still need to be able to load the new value of TCG_REG_TB.
+ * But this does still happen quite often.
+ */
if (tb_diff == (int16_t)tb_diff) {
-i1 = ADDI | TAI(TCG_REG_TB, TCG_REG_TB, tb_diff);
-i2 = B | (br_diff & 0x3fc);
+i[0] = ADDI | TAI(TCG_REG_TB, TCG_REG_TB, tb_diff);
+i[1] = B | (br_diff & 0x3fc);
+i[2] = MTSPR | RS(TCG_REG_TB) | CTR;
+
} else {
intptr_t lo = (int16_t)tb_diff;
intptr_t hi = (int32_t)(tb_diff - lo);
assert(tb_diff == hi + lo);
-i1 = ADDIS | TAI(TCG_REG_TB, TCG_REG_TB, hi >> 16);
-i2 = ADDI | TAI(TCG_REG_TB, TCG_REG_TB, lo);
+i[0] = ADDIS | TAI(TCG_REG_TB, TCG_REG_TB, hi >> 16);
+i[1] = ADDI | TAI(TCG_REG_TB, TCG_REG_TB, lo);
+
+br_diff -= 4;
+if (have_isa_2_07 && in_range_b(br_diff)) {
+i[2] = B | (br_diff & 0x3fc);
+} else {
+i[2] = MTSPR | RS(TCG_REG_TB) | CTR;
+}
}
-#if HOST_BIG_ENDIAN
-pair = (uint64_t)i1 << 32 | i2;
-#else
-pair = (uint64_t)i2 << 32 | i1;
-#endif
-/* As per the enclosing if, this is ppc64. Avoid the _Static_assert
- within qatomic_set that would fail to build a ppc32 host. */
-qatomic_set__nocheck((uint64_t *)jmp_rw, pair);
-flush_idcache_range(jmp_rx, jmp_rw, 8);
+i[3] = BCCTR | BO_ALWAYS;
+ppc64_replace_insns(jmp_rx, jmp_rw, i);
+
} else {
intptr_t diff = addr - jmp_rx;
tcg_debug_assert(in_range_b(diff));
--
2.25.1
Re: [PATCH v3 03/20] ppc4xx_sdram: Get rid of the init RAM hack
On Wed, 14 Sep 2022, BALATON Zoltan wrote:
On Wed, 14 Sep 2022, Cédric Le Goater wrote:
On 9/14/22 13:44, BALATON Zoltan wrote:
On Wed, 14 Sep 2022, Cédric Le Goater wrote:
On 9/13/22 21:52, BALATON Zoltan wrote:
The do_init parameter of ppc4xx_sdram_init() is used to map memory
regions that is normally done by the firmware by programming the SDRAM
controller. This is needed when booting a kernel directly from -kernel
without a firmware. Do this from board code accesing normal SDRAM
accessing
Fixed, also two ofhers in another patch you haven't noticed.
controller registers the same way as firmware would do, so we can get
rid of this hack.
Signed-off-by: BALATON Zoltan
---
v2: Fix ref405ep boot with -kernel and U-Boot
hw/ppc/ppc405.h | 1 -
hw/ppc/ppc405_boards.c | 12 ++--
hw/ppc/ppc405_uc.c | 4 +---
hw/ppc/ppc440_bamboo.c | 8 +++-
hw/ppc/ppc440_uc.c | 2 --
hw/ppc/ppc4xx_devs.c | 11 +--
include/hw/ppc/ppc4xx.h | 8 ++--
7 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index 1e558c7831..756865621b 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -169,7 +169,6 @@ struct Ppc405SoCState {
/* Public */
MemoryRegion ram_banks[2];
hwaddr ram_bases[2], ram_sizes[2];
- bool do_dram_init;
MemoryRegion *dram_mr;
hwaddr ram_size;
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 083f12b23e..bf02a71c6d 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -274,6 +274,7 @@ static void ppc405_init(MachineState *machine)
MachineClass *mc = MACHINE_GET_CLASS(machine);
const char *kernel_filename = machine->kernel_filename;
MemoryRegion *sysmem = get_system_memory();
+ CPUPPCState *env;
if (machine->ram_size != mc->default_ram_size) {
char *sz = size_to_str(mc->default_ram_size);
@@ -288,12 +289,19 @@ static void ppc405_init(MachineState *machine)
machine->ram_size, _fatal);
object_property_set_link(OBJECT(>soc), "dram",
OBJECT(machine->ram), _abort);
- object_property_set_bool(OBJECT(>soc), "dram-init",
- kernel_filename != NULL, _abort);
object_property_set_uint(OBJECT(>soc), "sys-clk",
,
_abort);
qdev_realize(DEVICE(>soc), NULL, _fatal);
+ /* Enable SDRAM memory regions */
+ /* FIXME This shouldn't be needed with firmware but we lack SPD
data */
what do you mean ?
U-Boot detects the available RAM by reading the SPD info of the RAM
modules but that probably also needs i2c emulation. See sam460ex.
+ env = >soc.cpu.env;
+ if (ppc_dcr_write(env->dcr_env, SDRAM0_CFGADDR, 0x20) ||
+ ppc_dcr_write(env->dcr_env, SDRAM0_CFGDATA, 0x8000)) {
I am not in favor of these ppc_drc_write calls and this is still a hack.
It's not. Normally this is done by firmware to enable memory controller
but the board code has to do it if not using firmware (e.g. booting with
-kernel) the same way it provides bootinfo or device tree mods the
firmware would normally do or in this case maybe the emulation is
incomplete so the part of firmware that configures the SDRAM controller
does not run.
Exactly, and what the above proposal does is mimicking execution of CPU
instructions before the CPU is even fully initiated. Reset has not been
called at that stage.
I don't get this. We're not calling any CPU instructions, ppc_dcr_write just
calls the write callback the device has registered for the dcr so it just
does the same as the hack did at the end just doing it the same way the
firmware should do.
The "dram-init" property is a cleaner solution. It takes care of doing
the
pre-mapping of RAM banks in the realize routine of the sdram model (when
available).
I disagree, the hardware does not have such feature, it proviesd DCRs as
the way to configure it. Adding a special property for it deviates from
hardware and clutters qtree.
In this machine, running QEMU with -kernel deviates from HW. That's
In all machines booting with -kernel likely deviates and all machines
probably have additinal code in this case to do some things normally done by
the firmware. Look at pegasos2_machine_reset() for example. All that is not
needed when we boot with firmware as then the firmware will do all that and
provide the device tree, etc. bur we need to do these when booting without
firmware. In thes case QEMU also emulates the firmware and has to do thinigs
like enabling the memory controller.
the whole purpose of this option. It assumes that the SDRAM device
is pre-initialized (and much more should be done) by the QEMU machine
and the simplest way to acheive this goal is to inform the SDRAM model
to take care of the pre-initialization.
In my opinion the SDRAM controller model should model the hardware and if the
board
Re: [PATCH] vfio/common: Do not g_free in vfio_get_iommu_info
On Wed, 14 Sep 2022 12:10:29 -0600
Alex Williamson wrote:
> On Mon, 12 Sep 2022 13:51:30 -0700
> Nicolin Chen wrote:
>
> > On Mon, Sep 12, 2022 at 02:38:52PM +0200, Cornelia Huck wrote:
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > On Fri, Sep 09 2022, Nicolin Chen wrote:
> > >
> > > > Its caller vfio_connect_container() assigns a default value
> > > > to info->iova_pgsizes, even if vfio_get_iommu_info() fails.
> > > > This would result in a "Segmentation fault" error, when the
> > > > VFIO_IOMMU_GET_INFO ioctl errors out.
> > > >
> > > > Since the caller has g_free already, drop the g_free in its
> > > > rollback routine and add a line of comments to highlight it.
> > >
> > > There's basically two ways to fix this:
> > >
> > > - return *info in any case, even on error
> > > - free *info on error, and make sure that the caller doesn't try to
> > > access *info if the function returned !0
> > >
> > > The problem with the first option is that the caller will access invalid
> > > information if it neglects to check the return code, and that might lead
> > > to not-that-obvious errors; in the second case, a broken caller would at
> > > least fail quickly with a segfault. The current code is easier to fix
> > > with the first option.
> > >
> > > I think I'd prefer the second option; but obviously maintainer's choice.
> > >
> >
> > The caller does check rc all the time. So I made a smaller fix
> > (the first option). Attaching the git-diff for the second one.
> >
> > Alex, please let me know which one you prefer. Thanks!
> >
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index 51b2e05c76..74431411ab 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> [snip]
>
> I think we can do better than that, I don't think we need to maintain
> the existing grouping, and that FIXME comment is outdated and has
> drifted from the relevant line of code. What about:
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index ace9562a9ba1..8d8c54d59083 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -2111,29 +2111,31 @@ static int vfio_connect_container(VFIOGroup *group,
> AddressSpace *as,
> {
> struct vfio_iommu_type1_info *info;
>
> -/*
> - * FIXME: This assumes that a Type1 IOMMU can map any 64-bit
> - * IOVA whatsoever. That's not actually true, but the current
> - * kernel interface doesn't tell us what it can map, and the
> - * existing Type1 IOMMUs generally support any IOVA we're
> - * going to actually try in practice.
> + /*
> + * Setup defaults for container pgsizes and dma_max_mappings if not
> + * provided by kernel below.
> */
> -ret = vfio_get_iommu_info(container, );
> -
> -if (ret || !(info->flags & VFIO_IOMMU_INFO_PGSIZES)) {
> -/* Assume 4k IOVA page size */
> -info->iova_pgsizes = 4096;
> -}
> -vfio_host_win_add(container, 0, (hwaddr)-1, info->iova_pgsizes);
> -container->pgsizes = info->iova_pgsizes;
> -
> -/* The default in the kernel ("dma_entry_limit") is 65535. */
> +container->pgsizes = 4096;
> container->dma_max_mappings = 65535;
> +
> +ret = vfio_get_iommu_info(container, );
> if (!ret) {
Or better still:
if (!vfio_get_iommu_info(container, )) {
> +if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
> +container->pgsizes = info->iova_pgsizes;
> +}
> +
> vfio_get_info_dma_avail(info, >dma_max_mappings);
> vfio_get_iommu_info_migration(container, info);
> +g_free(info);
> }
> -g_free(info);
> +
> +/*
> + * FIXME: We should parse VFIO_IOMMU_TYPE1_INFO_CAP_IOVA_RANGE
> + * information to get the actual window extent rather than assume
> + * a 64-bit IOVA address space.
> + */
> +vfio_host_win_add(container, 0, (hwaddr)-1, container->pgsizes);
> +
> break;
> }
> case VFIO_SPAPR_TCE_v2_IOMMU:
>
Re: [PATCH v2 4/4] scripts: add script to compare compatible properties
On 9/14/22 13:55, Maksim Davydov wrote: This script run QEMU to obtain compat_props of machines and default values of different types and produce appropriate table. This table can be used to compare machine types to choose the most suitable machine. Also table in json or csv format should be used to check that new machine doesn't affect previous ones via comparisin tables with and without new machine. Default values of properties are needed to fill "holes" in the table (one machine has these properties and another not). Notes: * some init values from the devices can't be available like properties from virtio-9p when configure has --disable-virtfs. This situations will be seen in the table as "unavailable driver". * Default values can be get can be obtained in an unobvious way, like x86 features. If the script doesn't know how to get property default value to compare one machine with another it fills "holes" with "unavailable method". This is done because script uses whitelist model to get default values of different types. It means that the method that can't be applied to a new type that can crash this script. It is better to get an "unavailable driver" when creating a new machine with new compatible properties than to break this script. So it turns out a more stable and generic script. * If the default value can't be obtained because this property doesn't exist or because this property can't have default value, appropriate "hole" will be filled by "unknown property" or "no default value" * If the property is applied to the abstract class, the script collects default values from all child classes (set of default values) Example: ./scripts/compare_mt.py --mt pc-q35-3.1 pc-q35-2.12 ╒╤═══╤═══╕ ││ pc-q35-2.12 │ pc-q35-3.1 │ ╞╪═══╪═══╡ │EPYC-IBPB-x86_64-cpu-xlevel │ 0x800a │ 2147483678 │ ├┼───┼───┤ │ EPYC-x86_64-cpu-xlevel │ 0x800a │ 2147483678 │ ├┼───┼───┤ │ Skylake-Server-IBRS-x86_64-cpu-pku │ False │ True │ ├┼───┼───┤ │ Skylake-Server-x86_64-cpu-pku│ False │ True │ ├┼───┼───┤ │ VGA-global-vmstate │ True │ False │ ├┼───┼───┤ │ cirrus-vga-global-vmstate │ True │ False │ ├┼───┼───┤ │hda-audio-use-timer │ False │ True │ ├┼───┼───┤ │ migration-decompress-error-check │ False │ True │ ├┼───┼───┤ │ qxl-vga-global-vmstate │ True │ False │ ├┼───┼───┤ │ vmware-svga-global-vmstate │ True │ False │ ├┼───┼───┤ │ x86_64-cpu-legacy-cache │ True │ [True, False] │ ├┼───┼───┤ │ x86_64-cpu-topoext │ False │ [True, False] │ ├┼───┼───┤ │ x86_64-cpu-x-hv-synic-kvm-only │ True │ False │ ╘╧═══╧═══╛ Looks great! Some nitpicking: 1. I'd use left-align, at least for the first column as more readable. 2. Seems '-' is bad separator for driver-property, as both drivers and properties sometimes has own '-' inside. Let's use something else. Maybe colon, like driver:property. Or, maybe, just add one more column, to keep driver and property names in different columns. Hmm, and what means [True, False] ? Signed-off-by: Maksim Davydov --- scripts/compare_mt.py | 374 ++ 1 file changed, 374 insertions(+) create mode 100755 scripts/compare_mt.py diff --git a/scripts/compare_mt.py b/scripts/compare_mt.py new file mode 100755 index 00..1df5e6c4f4 --- /dev/null +++ b/scripts/compare_mt.py @@ -0,0 +1,374 @@ +#!/usr/bin/env python3 +# +# Script to compare machine type compatible properties (include/hw/boards.h). +# compat_props are applied to the driver during initialization to change +# default values, for instance, to maintain compatibility. +# This script constructs table with machines and values of their compat_props +# to compare and to find places for improvements or places with bugs. If +# during the comparision, some machine type doesn't have a
Re: [PATCH v3 03/20] ppc4xx_sdram: Get rid of the init RAM hack
On Wed, 14 Sep 2022, Cédric Le Goater wrote:
On 9/14/22 13:44, BALATON Zoltan wrote:
On Wed, 14 Sep 2022, Cédric Le Goater wrote:
On 9/13/22 21:52, BALATON Zoltan wrote:
The do_init parameter of ppc4xx_sdram_init() is used to map memory
regions that is normally done by the firmware by programming the SDRAM
controller. This is needed when booting a kernel directly from -kernel
without a firmware. Do this from board code accesing normal SDRAM
accessing
Fixed, also two ofhers in another patch you haven't noticed.
controller registers the same way as firmware would do, so we can get
rid of this hack.
Signed-off-by: BALATON Zoltan
---
v2: Fix ref405ep boot with -kernel and U-Boot
hw/ppc/ppc405.h | 1 -
hw/ppc/ppc405_boards.c | 12 ++--
hw/ppc/ppc405_uc.c | 4 +---
hw/ppc/ppc440_bamboo.c | 8 +++-
hw/ppc/ppc440_uc.c | 2 --
hw/ppc/ppc4xx_devs.c | 11 +--
include/hw/ppc/ppc4xx.h | 8 ++--
7 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index 1e558c7831..756865621b 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -169,7 +169,6 @@ struct Ppc405SoCState {
/* Public */
MemoryRegion ram_banks[2];
hwaddr ram_bases[2], ram_sizes[2];
- bool do_dram_init;
MemoryRegion *dram_mr;
hwaddr ram_size;
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 083f12b23e..bf02a71c6d 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -274,6 +274,7 @@ static void ppc405_init(MachineState *machine)
MachineClass *mc = MACHINE_GET_CLASS(machine);
const char *kernel_filename = machine->kernel_filename;
MemoryRegion *sysmem = get_system_memory();
+ CPUPPCState *env;
if (machine->ram_size != mc->default_ram_size) {
char *sz = size_to_str(mc->default_ram_size);
@@ -288,12 +289,19 @@ static void ppc405_init(MachineState *machine)
machine->ram_size, _fatal);
object_property_set_link(OBJECT(>soc), "dram",
OBJECT(machine->ram), _abort);
- object_property_set_bool(OBJECT(>soc), "dram-init",
- kernel_filename != NULL, _abort);
object_property_set_uint(OBJECT(>soc), "sys-clk", ,
_abort);
qdev_realize(DEVICE(>soc), NULL, _fatal);
+ /* Enable SDRAM memory regions */
+ /* FIXME This shouldn't be needed with firmware but we lack SPD data
*/
what do you mean ?
U-Boot detects the available RAM by reading the SPD info of the RAM modules
but that probably also needs i2c emulation. See sam460ex.
+ env = >soc.cpu.env;
+ if (ppc_dcr_write(env->dcr_env, SDRAM0_CFGADDR, 0x20) ||
+ ppc_dcr_write(env->dcr_env, SDRAM0_CFGDATA, 0x8000)) {
I am not in favor of these ppc_drc_write calls and this is still a hack.
It's not. Normally this is done by firmware to enable memory controller but
the board code has to do it if not using firmware (e.g. booting with
-kernel) the same way it provides bootinfo or device tree mods the firmware
would normally do or in this case maybe the emulation is incomplete so the
part of firmware that configures the SDRAM controller does not run.
Exactly, and what the above proposal does is mimicking execution of CPU
instructions before the CPU is even fully initiated. Reset has not been
called at that stage.
I don't get this. We're not calling any CPU instructions, ppc_dcr_write
just calls the write callback the device has registered for the dcr so it
just does the same as the hack did at the end just doing it the same way
the firmware should do.
The "dram-init" property is a cleaner solution. It takes care of doing the
pre-mapping of RAM banks in the realize routine of the sdram model (when
available).
I disagree, the hardware does not have such feature, it proviesd DCRs as
the way to configure it. Adding a special property for it deviates from
hardware and clutters qtree.
In this machine, running QEMU with -kernel deviates from HW. That's
In all machines booting with -kernel likely deviates and all machines
probably have additinal code in this case to do some things normally done
by the firmware. Look at pegasos2_machine_reset() for example. All that is
not needed when we boot with firmware as then the firmware will do all
that and provide the device tree, etc. bur we need to do these when
booting without firmware. In thes case QEMU also emulates the firmware
and has to do thinigs like enabling the memory controller.
the whole purpose of this option. It assumes that the SDRAM device
is pre-initialized (and much more should be done) by the QEMU machine
and the simplest way to acheive this goal is to inform the SDRAM model
to take care of the pre-initialization.
In my opinion the SDRAM controller model should model the hardware and if
the board uses it differently then it should take care of
Re: [PATCH] vfio/common: Do not g_free in vfio_get_iommu_info
On Mon, 12 Sep 2022 13:51:30 -0700
Nicolin Chen wrote:
> On Mon, Sep 12, 2022 at 02:38:52PM +0200, Cornelia Huck wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Fri, Sep 09 2022, Nicolin Chen wrote:
> >
> > > Its caller vfio_connect_container() assigns a default value
> > > to info->iova_pgsizes, even if vfio_get_iommu_info() fails.
> > > This would result in a "Segmentation fault" error, when the
> > > VFIO_IOMMU_GET_INFO ioctl errors out.
> > >
> > > Since the caller has g_free already, drop the g_free in its
> > > rollback routine and add a line of comments to highlight it.
> >
> > There's basically two ways to fix this:
> >
> > - return *info in any case, even on error
> > - free *info on error, and make sure that the caller doesn't try to
> > access *info if the function returned !0
> >
> > The problem with the first option is that the caller will access invalid
> > information if it neglects to check the return code, and that might lead
> > to not-that-obvious errors; in the second case, a broken caller would at
> > least fail quickly with a segfault. The current code is easier to fix
> > with the first option.
> >
> > I think I'd prefer the second option; but obviously maintainer's choice.
>
> The caller does check rc all the time. So I made a smaller fix
> (the first option). Attaching the git-diff for the second one.
>
> Alex, please let me know which one you prefer. Thanks!
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 51b2e05c76..74431411ab 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
[snip]
I think we can do better than that, I don't think we need to maintain
the existing grouping, and that FIXME comment is outdated and has
drifted from the relevant line of code. What about:
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index ace9562a9ba1..8d8c54d59083 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -2111,29 +2111,31 @@ static int vfio_connect_container(VFIOGroup *group,
AddressSpace *as,
{
struct vfio_iommu_type1_info *info;
-/*
- * FIXME: This assumes that a Type1 IOMMU can map any 64-bit
- * IOVA whatsoever. That's not actually true, but the current
- * kernel interface doesn't tell us what it can map, and the
- * existing Type1 IOMMUs generally support any IOVA we're
- * going to actually try in practice.
+ /*
+ * Setup defaults for container pgsizes and dma_max_mappings if not
+ * provided by kernel below.
*/
-ret = vfio_get_iommu_info(container, );
-
-if (ret || !(info->flags & VFIO_IOMMU_INFO_PGSIZES)) {
-/* Assume 4k IOVA page size */
-info->iova_pgsizes = 4096;
-}
-vfio_host_win_add(container, 0, (hwaddr)-1, info->iova_pgsizes);
-container->pgsizes = info->iova_pgsizes;
-
-/* The default in the kernel ("dma_entry_limit") is 65535. */
+container->pgsizes = 4096;
container->dma_max_mappings = 65535;
+
+ret = vfio_get_iommu_info(container, );
if (!ret) {
+if (info->flags & VFIO_IOMMU_INFO_PGSIZES) {
+container->pgsizes = info->iova_pgsizes;
+}
+
vfio_get_info_dma_avail(info, >dma_max_mappings);
vfio_get_iommu_info_migration(container, info);
+g_free(info);
}
-g_free(info);
+
+/*
+ * FIXME: We should parse VFIO_IOMMU_TYPE1_INFO_CAP_IOVA_RANGE
+ * information to get the actual window extent rather than assume
+ * a 64-bit IOVA address space.
+ */
+vfio_host_win_add(container, 0, (hwaddr)-1, container->pgsizes);
+
break;
}
case VFIO_SPAPR_TCE_v2_IOMMU:
Re: [PATCH 0/2] target/m68k: fix two writes to %sr
On Tue, Sep 13, 2022 at 6:29 PM Mark Cave-Ayland < mark.cave-ayl...@ilande.co.uk> wrote: > On 13/09/2022 15:28, Richard Henderson wrote: > > > The second was described by Mark in the lobby of KVM Forum. > > The first was found by inspection of other uses of gen_helper_set_sr. > > > > r~ > > > > Richard Henderson (2): > >target/m68k: Fix MACSR to CCR > >target/m68k: Perform writback before modifying SR > > > > target/m68k/translate.c | 14 +- > > 1 file changed, 9 insertions(+), 5 deletions(-) > > I've applied these on top of my MacOS virtual memory branch at > https://github.com/mcayland/qemu/commits/q800.upstream2-vm and I can > confirm that > MacOS 8.1 now boots here with virtual memory enabled :) > > Possibly it might be worth including a tidied-up version of the "WIP: > target/m68k: > always exit_tb when changing sr with andi/ori/eori" commit from that > branch which is > also related to switching between supervisor and user modes under MacOS. > Shall I tidy > it up and send it to the list? > > > ATB, > > Mark. > > I've compiled the branch mentioned above with a fully updated MSYS2 on windows. The executable hangs when running Mac OS 8 with Virtual Memory enabled. On a fast machine I see Error 7 as before, on a slower machine, the boot screen just hangs with no error shown. A Linux build does work, also on the slower machine. Best, Howard
Re: [PATCH 0/2] target/m68k: fix two writes to %sr
On Wed, Sep 14, 2022 at 6:48 PM Howard Spoelstra wrote: > > > On Tue, Sep 13, 2022 at 6:29 PM Mark Cave-Ayland < > mark.cave-ayl...@ilande.co.uk> wrote: > >> On 13/09/2022 15:28, Richard Henderson wrote: >> >> > The second was described by Mark in the lobby of KVM Forum. >> > The first was found by inspection of other uses of gen_helper_set_sr. >> > >> > r~ >> > >> > Richard Henderson (2): >> >target/m68k: Fix MACSR to CCR >> >target/m68k: Perform writback before modifying SR >> > >> > target/m68k/translate.c | 14 +- >> > 1 file changed, 9 insertions(+), 5 deletions(-) >> >> I've applied these on top of my MacOS virtual memory branch at >> https://github.com/mcayland/qemu/commits/q800.upstream2-vm and I can >> confirm that >> MacOS 8.1 now boots here with virtual memory enabled :) >> >> Possibly it might be worth including a tidied-up version of the "WIP: >> target/m68k: >> always exit_tb when changing sr with andi/ori/eori" commit from that >> branch which is >> also related to switching between supervisor and user modes under MacOS. >> Shall I tidy >> it up and send it to the list? >> >> >> ATB, >> >> Mark. >> >> > I've compiled the branch mentioned above with a fully updated MSYS2 on > windows. The executable hangs when running Mac OS 8 with Virtual Memory > enabled. On a fast machine I see Error 7 as before, on a slower machine, > the boot screen just hangs with no error shown. A Linux build does work, > also on the slower machine. > > Best, > Howard > ps: a debug enabled non-stripped build will run with Virtual Memory enabled on Windows.
Re: [PATCH 28/37] target/i386: reimplement 0x0f 0x38, add AVX
On Tue, Sep 13, 2022 at 11:31 AM Richard Henderson
wrote:
> > +void glue(helper_vpmaskmovq_st, SUFFIX)(CPUX86State *env,
> > +Reg *v, Reg *s, target_ulong a0)
> > +{
> > +int i;
> > +
> > +for (i = 0; i < (1 << SHIFT); i++) {
> > +if (v->Q(i) >> 63) {
> > +cpu_stq_data_ra(env, a0 + i * 8, s->Q(i), GETPC());
> > +}
> > +}
> > +}
>
> Any idea if hw will write incomplete data if the pieces cross page
> boundaries, and the
> second page is invalid? We're not good at that for any other vector sized
> write, though,
> so not critical.
No, I will check.
> > +void glue(helper_vpgatherdd, SUFFIX)(CPUX86State *env,
> > +Reg *d, Reg *v, Reg *s, target_ulong a0, unsigned scale)
> > +{
> > +int i;
> > +for (i = 0; i < (2 << SHIFT); i++) {
> > +if (v->L(i) >> 31) {
> > +target_ulong addr = a0
> > ++ ((target_ulong)(int32_t)s->L(i) << scale);
> > +d->L(i) = cpu_ldl_data_ra(env, addr, GETPC());
> > +}
> > +v->L(i) = 0;
> > +}
> > +}
>
> Better to not modify registers until all potential #GP are raised.
This is actually intentional: elements of v are zeroes whenever an
element is read successfully, so that values are not reread when the
instruction restarts. The manual says "If a fault is triggered by an
element and delivered, all elements closer to the LSB of the
destination zmm will be completed".
Paolo
Re: [PATCH] target/m68k: Implement atomic test-and-set
Le 29/08/2022 à 07:17, Richard Henderson a écrit :
This is slightly more complicated than cas,
because tas is allowed on data registers.
Signed-off-by: Richard Henderson
---
target/m68k/translate.c | 40 ++--
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 8f3c298ad0..0aef827b38 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -2825,19 +2825,39 @@ DISAS_INSN(illegal)
gen_exception(s, s->base.pc_next, EXCP_ILLEGAL);
}
-/* ??? This should be atomic. */
DISAS_INSN(tas)
{
-TCGv dest;
-TCGv src1;
-TCGv addr;
+int mode = extract32(insn, 3, 3);
+int reg0 = REG(insn, 0);
-dest = tcg_temp_new();
-SRC_EA(env, src1, OS_BYTE, 1, );
-gen_logic_cc(s, src1, OS_BYTE);
-tcg_gen_ori_i32(dest, src1, 0x80);
-DEST_EA(env, insn, OS_BYTE, dest, );
-tcg_temp_free(dest);
+if (mode == 0) {
+/* data register direct */
+TCGv dest = cpu_dregs[reg0];
+gen_logic_cc(s, dest, OS_BYTE);
+tcg_gen_ori_tl(dest, dest, 0x80);
+} else {
+TCGv src1, addr;
+
+addr = gen_lea_mode(env, s, mode, reg0, OS_BYTE);
+if (IS_NULL_QREG(addr)) {
+gen_addr_fault(s);
+return;
+}
+src1 = tcg_temp_new();
+tcg_gen_atomic_fetch_or_tl(src1, addr, tcg_constant_tl(0x80),
+ IS_USER(s), MO_SB);
+gen_logic_cc(s, src1, OS_BYTE);
+tcg_temp_free(src1);
+
+switch (mode) {
+case 3: /* Indirect postincrement. */
+tcg_gen_addi_i32(AREG(insn, 0), addr, 1);
+break;
+case 4: /* Indirect predecrememnt. */
+tcg_gen_mov_i32(AREG(insn, 0), addr);
+break;
+}
+}
}
DISAS_INSN(mull)
Reviewed-by: Laurent Vivier
[RFC PATCH 2/4] qtest: make read/write operation appear to be from CPU
The point of qtest is to simulate how running code might interact with
the system. However because it's not a real system we have places in
the code which especially handle check qtest_enabled() before
referencing current_cpu. Now we can encode these details in the
MemTxAttrs lets do that so we can start removing them.
Signed-off-by: Alex Bennée
---
softmmu/qtest.c | 31 +++
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index f8acef2628..c086bd34b7 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -362,6 +362,13 @@ static void qtest_clock_warp(int64_t dest)
qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
}
+/*
+ * QTest memory accesses are treated as though they come from the
+ * first (non-existent) CPU. We need to expose this via MemTxAttrs for
+ * those bits of HW which care which core is accessing them.
+ */
+#define MEMTXATTRS_QTEST ((MemTxAttrs) { .requester_cpu = 1 })
+
static void qtest_process_command(CharBackend *chr, gchar **words)
{
const gchar *command;
@@ -525,17 +532,17 @@ static void qtest_process_command(CharBackend *chr, gchar
**words)
} else if (words[0][5] == 'w') {
uint16_t data = value;
tswap16s();
-address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+address_space_write(first_cpu->as, addr, MEMTXATTRS_QTEST,
, 2);
} else if (words[0][5] == 'l') {
uint32_t data = value;
tswap32s();
-address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+address_space_write(first_cpu->as, addr, MEMTXATTRS_QTEST,
, 4);
} else if (words[0][5] == 'q') {
uint64_t data = value;
tswap64s();
-address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+address_space_write(first_cpu->as, addr, MEMTXATTRS_QTEST,
, 8);
}
qtest_send_prefix(chr);
@@ -554,21 +561,21 @@ static void qtest_process_command(CharBackend *chr, gchar
**words)
if (words[0][4] == 'b') {
uint8_t data;
-address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+address_space_read(first_cpu->as, addr, MEMTXATTRS_QTEST,
, 1);
value = data;
} else if (words[0][4] == 'w') {
uint16_t data;
-address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+address_space_read(first_cpu->as, addr, MEMTXATTRS_QTEST,
, 2);
value = tswap16(data);
} else if (words[0][4] == 'l') {
uint32_t data;
-address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+address_space_read(first_cpu->as, addr, MEMTXATTRS_QTEST,
, 4);
value = tswap32(data);
} else if (words[0][4] == 'q') {
-address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+address_space_read(first_cpu->as, addr, MEMTXATTRS_QTEST,
, 8);
tswap64s();
}
@@ -589,7 +596,7 @@ static void qtest_process_command(CharBackend *chr, gchar
**words)
g_assert(len);
data = g_malloc(len);
-address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
+address_space_read(first_cpu->as, addr, MEMTXATTRS_QTEST, data,
len);
enc = g_malloc(2 * len + 1);
@@ -615,7 +622,7 @@ static void qtest_process_command(CharBackend *chr, gchar
**words)
g_assert(ret == 0);
data = g_malloc(len);
-address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
+address_space_read(first_cpu->as, addr, MEMTXATTRS_QTEST, data,
len);
b64_data = g_base64_encode(data, len);
qtest_send_prefix(chr);
@@ -650,7 +657,7 @@ static void qtest_process_command(CharBackend *chr, gchar
**words)
data[i] = 0;
}
}
-address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
+address_space_write(first_cpu->as, addr, MEMTXATTRS_QTEST, data,
len);
g_free(data);
@@ -673,7 +680,7 @@ static void qtest_process_command(CharBackend *chr, gchar
**words)
if (len) {
data = g_malloc(len);
memset(data, pattern, len);
-address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
+address_space_write(first_cpu->as, addr, MEMTXATTRS_QTEST,
data, len);
g_free(data);
}
@@ -707,7 +714,7 @@ static void qtest_process_command(CharBackend *chr, gchar
**words)
out_len = MIN(out_len, len);
Re: [PATCH 23/37] target/i386: reimplement 0x0f 0x78-0x7f, add AVX
On Mon, Sep 12, 2022 at 3:56 PM Richard Henderson
wrote:
> > +static void gen_SSE4a_I(DisasContext *s, CPUX86State *env, X86DecodedInsn
> > *decode)
> > +{
> > +TCGv_i32 length = tcg_const_i32(decode->immediate & 255);
> > +TCGv_i32 index = tcg_const_i32(decode->immediate >> 8);
> > +
> > +if (s->prefix & PREFIX_DATA) {
> > +gen_helper_extrq_i(cpu_env, s->ptr0, index, length);
> > +} else {
> > +gen_helper_insertq_i(cpu_env, s->ptr0, index, length);
> > +}
> > +tcg_temp_free_i32(length);
> > +tcg_temp_free_i32(index);
>
> Again, why the choice of delayed decode? I guess it doesn't matter, but it's
> odd.
Mostly because I wasn't sure of which would be preferable so I tried
different things. I think now I have a better picture.
I will mostly switch to decode_by_prefix.
Paolo
[PATCH v2 23/30] tests/docker: update and flatten debian-all-test-cross
Update to the latest stable Debian. While we are at it flatten into a single dockerfile. We also need to ensure we install clang as it is used for those builds as well. It would be nice to port this to lcitool but for now this will do. Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20220826172128.353798-18-alex.ben...@linaro.org> --- v2 - move ccache/clang/git/ninja-build to main insall stanza - minor comment tweaks --- .gitlab-ci.d/container-cross.yml | 1 - tests/docker/Makefile.include | 1 - .../dockerfiles/debian-all-test-cross.docker | 18 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml index 3a8bd75473..091c0d8fcb 100644 --- a/.gitlab-ci.d/container-cross.yml +++ b/.gitlab-ci.d/container-cross.yml @@ -14,7 +14,6 @@ amd64-debian-cross-container: amd64-debian-user-cross-container: extends: .container_job_template stage: containers - needs: ['amd64-debian10-container'] variables: NAME: debian-all-test-cross diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 24cd44e667..ddcc502049 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -129,7 +129,6 @@ docker-image-debian-nios2-cross: $(DOCKER_FILES_DIR)/debian-toolchain.docker \ $(call debian-toolchain, $@) # Specialist build images, sometimes very limited tools -docker-image-debian-all-test-cross: docker-image-debian10 docker-image-debian-loongarch-cross: docker-image-debian11 docker-image-debian-microblaze-cross: docker-image-debian10 docker-image-debian-nios2-cross: docker-image-debian10 diff --git a/tests/docker/dockerfiles/debian-all-test-cross.docker b/tests/docker/dockerfiles/debian-all-test-cross.docker index dedcea58b4..2beb077fb4 100644 --- a/tests/docker/dockerfiles/debian-all-test-cross.docker +++ b/tests/docker/dockerfiles/debian-all-test-cross.docker @@ -6,16 +6,24 @@ # basic compilers for as many targets as possible. We shall use this # to build and run linux-user tests on GitLab # -FROM qemu/debian10 +FROM docker.io/library/debian:11-slim -# What we need to build QEMU itself -RUN apt update && \ -DEBIAN_FRONTEND=noninteractive eatmydata \ +# Duplicate deb line as deb-src +RUN cat /etc/apt/sources.list | sed "s/^deb\ /deb-src /" >> /etc/apt/sources.list + +RUN export DEBIAN_FRONTEND=noninteractive && \ +apt-get update && \ +apt-get install -y eatmydata && \ +eatmydata apt-get dist-upgrade -y && \ apt build-dep -yy qemu -# Add the foreign architecture we want and install dependencies +# Add extra build tools and as many cross compilers as we can for testing RUN DEBIAN_FRONTEND=noninteractive eatmydata \ apt install -y --no-install-recommends \ +ccache \ +clang \ +git \ +ninja-build \ gcc-aarch64-linux-gnu \ libc6-dev-arm64-cross \ gcc-alpha-linux-gnu \ -- 2.34.1
[RFC PATCH 0/4] use MemTxAttrs to signal cpu index
Hi, This ostensibly fixes a bug with gdbstub when accessing the GIC state (via system registers). It also proposes a pattern for removing current_cpu/qtest_enabled() hacks in hw/ by passing the cpu index via MemTxAttrs. For the ARM timer code we also assert that those accesses do come from a CPU (as opposed to HW DMA attempting to overwrite stuff). We do need to audit helpers in target/ which use the address_space API to read and write data and ensure they properly form their MxTxAttrs so this works cleanly. What do people think? Alex Bennée (4): hw: encode accessing CPU index in MemTxAttrs qtest: make read/write operation appear to be from CPU hw/intc/gic: use MxTxAttrs to divine accessing CPU hw/timer: convert mptimer access to attrs to derive cpu index include/exec/memattrs.h | 4 +++- accel/tcg/cputlb.c | 22 -- hw/core/cpu-sysemu.c| 17 + hw/intc/arm_gic.c | 39 ++- hw/timer/arm_mptimer.c | 25 ++--- softmmu/qtest.c | 31 +++ 6 files changed, 87 insertions(+), 51 deletions(-) -- 2.34.1
[PATCH v2 18/30] configure: explicitly set cflags for --disable-pie
This is working around current limitation of Meson's handling of --disable-pie. Signed-off-by: Alex Bennée Cc: Paolo Bonzini --- configure | 6 ++ 1 file changed, 6 insertions(+) diff --git a/configure b/configure index 575dde1c1f..6c169b23b5 100755 --- a/configure +++ b/configure @@ -1394,6 +1394,12 @@ else pie="no" fi +# Meson currently only handles pie as a boolean for now so if we have +# explicitly disabled PIE we need to extend our cflags because it wont. +if test "$pie" = "no"; then +QEMU_CFLAGS="-fno-pie -no-pie $QEMU_CFLAGS" +fi + # Detect support for PT_GNU_RELRO + DT_BIND_NOW. # The combination is known as "full relro", because .got.plt is read-only too. if compile_prog "" "-Wl,-z,relro -Wl,-z,now" ; then -- 2.34.1
[RFC PATCH 4/4] hw/timer: convert mptimer access to attrs to derive cpu index
This removes the hacks to deal with empty current_cpu.
Signed-off-by: Alex Bennée
---
hw/timer/arm_mptimer.c | 25 ++---
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c
index cdfca3000b..a7fe6ddc0d 100644
--- a/hw/timer/arm_mptimer.c
+++ b/hw/timer/arm_mptimer.c
@@ -41,9 +41,10 @@
* which is used in both the ARM11MPCore and Cortex-A9MP.
*/
-static inline int get_current_cpu(ARMMPTimerState *s)
+static inline int get_current_cpu(ARMMPTimerState *s, MemTxAttrs attrs)
{
-int cpu_id = current_cpu ? current_cpu->cpu_index : 0;
+int cpu_id = attrs.requester_id;
+g_assert(attrs.requester_cpu == 1);
if (cpu_id >= s->num_cpu) {
hw_error("arm_mptimer: num-cpu %d but this cpu is %d!\n",
@@ -178,25 +179,27 @@ static void timerblock_write(void *opaque, hwaddr addr,
/* Wrapper functions to implement the "read timer/watchdog for
* the current CPU" memory regions.
*/
-static uint64_t arm_thistimer_read(void *opaque, hwaddr addr,
- unsigned size)
+static MemTxResult arm_thistimer_read(void *opaque, hwaddr addr, uint64_t
*data,
+ unsigned size, MemTxAttrs attrs)
{
ARMMPTimerState *s = (ARMMPTimerState *)opaque;
-int id = get_current_cpu(s);
-return timerblock_read(>timerblock[id], addr, size);
+int id = get_current_cpu(s, attrs);
+*data = timerblock_read(>timerblock[id], addr, size);
+return MEMTX_OK;
}
-static void arm_thistimer_write(void *opaque, hwaddr addr,
-uint64_t value, unsigned size)
+static MemTxResult arm_thistimer_write(void *opaque, hwaddr addr,
+uint64_t value, unsigned size, MemTxAttrs
attrs)
{
ARMMPTimerState *s = (ARMMPTimerState *)opaque;
-int id = get_current_cpu(s);
+int id = get_current_cpu(s, attrs);
timerblock_write(>timerblock[id], addr, value, size);
+return MEMTX_OK;
}
static const MemoryRegionOps arm_thistimer_ops = {
-.read = arm_thistimer_read,
-.write = arm_thistimer_write,
+.read_with_attrs = arm_thistimer_read,
+.write_with_attrs = arm_thistimer_write,
.valid = {
.min_access_size = 4,
.max_access_size = 4,
--
2.34.1
[PATCH v2 22/30] tests/docker: flatten debian-riscv64-test-cross
Flatten into a single dockerfile and update to match the rest of the test cross compile dockerfiles. Signed-off-by: Alex Bennée Message-Id: <20220826172128.353798-17-alex.ben...@linaro.org> --- v2 - minor reword of commit msg --- .gitlab-ci.d/container-cross.yml | 1 - tests/docker/Makefile.include | 1 - .../dockerfiles/debian-riscv64-test-cross.docker | 10 ++ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml index 95d57e1c5d..3a8bd75473 100644 --- a/.gitlab-ci.d/container-cross.yml +++ b/.gitlab-ci.d/container-cross.yml @@ -122,7 +122,6 @@ riscv64-debian-cross-container: riscv64-debian-test-cross-container: extends: .container_job_template stage: containers - needs: ['amd64-debian11-container'] variables: NAME: debian-riscv64-test-cross diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index b1bf56434f..24cd44e667 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -133,7 +133,6 @@ docker-image-debian-all-test-cross: docker-image-debian10 docker-image-debian-loongarch-cross: docker-image-debian11 docker-image-debian-microblaze-cross: docker-image-debian10 docker-image-debian-nios2-cross: docker-image-debian10 -docker-image-debian-riscv64-test-cross: docker-image-debian11 # These images may be good enough for building tests but not for test builds DOCKER_PARTIAL_IMAGES += debian-alpha-cross diff --git a/tests/docker/dockerfiles/debian-riscv64-test-cross.docker b/tests/docker/dockerfiles/debian-riscv64-test-cross.docker index 1d90901298..e5f83a5aeb 100644 --- a/tests/docker/dockerfiles/debian-riscv64-test-cross.docker +++ b/tests/docker/dockerfiles/debian-riscv64-test-cross.docker @@ -3,10 +3,12 @@ # # This docker target builds on the Debian Bullseye base image. # -FROM qemu/debian11 +FROM docker.io/library/debian:11-slim -RUN apt update && \ -DEBIAN_FRONTEND=noninteractive eatmydata \ -apt install -y --no-install-recommends \ +RUN export DEBIAN_FRONTEND=noninteractive && \ +apt-get update && \ +apt-get install -y eatmydata && \ +eatmydata apt-get dist-upgrade -y && \ +eatmydata apt-get install --no-install-recommends -y \ gcc-riscv64-linux-gnu \ libc6-dev-riscv64-cross -- 2.34.1
[RFC PATCH 1/4] hw: encode accessing CPU index in MemTxAttrs
We currently have hacks across the hw/ to reference current_cpu to
work out what the current accessing CPU is. This breaks in some cases
including using gdbstub to access HW state. As we have MemTxAttrs to
describe details about the access lets extend it to mention if this is
a CPU access and which one it is.
There are a number of places we need to fix up including:
CPU helpers directly calling address_space_*() fns
models in hw/ fishing the data out of current_cpu
I'll start addressing some of these in following patches.
Signed-off-by: Alex Bennée
---
include/exec/memattrs.h | 4 +++-
accel/tcg/cputlb.c | 22 --
hw/core/cpu-sysemu.c| 17 +
3 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index 9fb98bc1ef..3bccd5d291 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -43,7 +43,9 @@ typedef struct MemTxAttrs {
* (see MEMTX_ACCESS_ERROR).
*/
unsigned int memory:1;
-/* Requester ID (for MSI for example) */
+/* Requester is CPU (or as CPU, e.g. debug) */
+unsigned int requester_cpu:1;
+/* Requester ID (for MSI for example) or cpu_index */
unsigned int requester_id:16;
/* Invert endianness for this page */
unsigned int byte_swap:1;
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 8fad2d9b83..68dc7dc646 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1340,8 +1340,13 @@ static uint64_t io_readx(CPUArchState *env,
CPUIOTLBEntry *iotlbentry,
uint64_t val;
bool locked = false;
MemTxResult r;
+MemTxAttrs attrs = iotlbentry->attrs;
-section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs);
+/* encode the accessing CPU */
+attrs.requester_cpu = 1;
+attrs.requester_id = cpu->cpu_index;
+
+section = iotlb_to_section(cpu, iotlbentry->addr, attrs);
mr = section->mr;
mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr;
cpu->mem_io_pc = retaddr;
@@ -1353,14 +1358,14 @@ static uint64_t io_readx(CPUArchState *env,
CPUIOTLBEntry *iotlbentry,
qemu_mutex_lock_iothread();
locked = true;
}
-r = memory_region_dispatch_read(mr, mr_offset, , op,
iotlbentry->attrs);
+r = memory_region_dispatch_read(mr, mr_offset, , op, attrs);
if (r != MEMTX_OK) {
hwaddr physaddr = mr_offset +
section->offset_within_address_space -
section->offset_within_region;
cpu_transaction_failed(cpu, physaddr, addr, memop_size(op),
access_type,
- mmu_idx, iotlbentry->attrs, r, retaddr);
+ mmu_idx, attrs, r, retaddr);
}
if (locked) {
qemu_mutex_unlock_iothread();
@@ -1395,8 +1400,13 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry
*iotlbentry,
MemoryRegion *mr;
bool locked = false;
MemTxResult r;
+MemTxAttrs attrs = iotlbentry->attrs;
+
+/* encode the accessing CPU */
+attrs.requester_cpu = 1;
+attrs.requester_id = cpu->cpu_index;
-section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs);
+section = iotlb_to_section(cpu, iotlbentry->addr, attrs);
mr = section->mr;
mr_offset = (iotlbentry->addr & TARGET_PAGE_MASK) + addr;
if (!cpu->can_do_io) {
@@ -1414,14 +1424,14 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry
*iotlbentry,
qemu_mutex_lock_iothread();
locked = true;
}
-r = memory_region_dispatch_write(mr, mr_offset, val, op,
iotlbentry->attrs);
+r = memory_region_dispatch_write(mr, mr_offset, val, op, attrs);
if (r != MEMTX_OK) {
hwaddr physaddr = mr_offset +
section->offset_within_address_space -
section->offset_within_region;
cpu_transaction_failed(cpu, physaddr, addr, memop_size(op),
- MMU_DATA_STORE, mmu_idx, iotlbentry->attrs, r,
+ MMU_DATA_STORE, mmu_idx, attrs, r,
retaddr);
}
if (locked) {
diff --git a/hw/core/cpu-sysemu.c b/hw/core/cpu-sysemu.c
index 00253f8929..bd7ae983ed 100644
--- a/hw/core/cpu-sysemu.c
+++ b/hw/core/cpu-sysemu.c
@@ -51,13 +51,22 @@ hwaddr cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr
addr,
MemTxAttrs *attrs)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
+MemTxAttrs local = { };
+hwaddr res;
if (cc->sysemu_ops->get_phys_page_attrs_debug) {
-return cc->sysemu_ops->get_phys_page_attrs_debug(cpu, addr, attrs);
+res = cc->sysemu_ops->get_phys_page_attrs_debug(cpu, addr, );
+} else {
+/* Fallback for CPUs which don't implement the _attrs_ hook */
+local = MEMTXATTRS_UNSPECIFIED;
+res = cc->sysemu_ops->get_phys_page_debug(cpu, addr);
}
-/* Fallback for CPUs which don't implement the _attrs_ hook */
-*attrs =
[PATCH v2 11/30] tests/docker: update and flatten debian-mips64-cross
Update to the latest stable Debian. While we are at it flatten into a single dockerfile. We really don't need the rest of the stuff from the QEMU base image just to compile test images. Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20220826172128.353798-7-alex.ben...@linaro.org> --- .gitlab-ci.d/container-cross.yml| 1 - tests/docker/Makefile.include | 1 - tests/docker/dockerfiles/debian-mips64-cross.docker | 12 +++- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml index 15a5270f6d..a3bfa483bf 100644 --- a/.gitlab-ci.d/container-cross.yml +++ b/.gitlab-ci.d/container-cross.yml @@ -77,7 +77,6 @@ m68k-debian-cross-container: mips64-debian-cross-container: extends: .container_job_template stage: containers - needs: ['amd64-debian10-container'] variables: NAME: debian-mips64-cross diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 95790e974e..6c2ee3b175 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -89,7 +89,6 @@ DOCKER_PARTIAL_IMAGES += fedora endif docker-image-debian-mips-cross: docker-image-debian10 -docker-image-debian-mips64-cross: docker-image-debian10 docker-image-debian-sh4-cross: docker-image-debian10 docker-image-debian-sparc64-cross: docker-image-debian10 diff --git a/tests/docker/dockerfiles/debian-mips64-cross.docker b/tests/docker/dockerfiles/debian-mips64-cross.docker index 09c2ba584e..afcff9726f 100644 --- a/tests/docker/dockerfiles/debian-mips64-cross.docker +++ b/tests/docker/dockerfiles/debian-mips64-cross.docker @@ -1,12 +1,14 @@ # # Docker cross-compiler target # -# This docker target builds on the debian Buster base image. +# This docker target builds on the Debian Bullseye base image. # -FROM qemu/debian10 +FROM docker.io/library/debian:11-slim -RUN apt update && \ -DEBIAN_FRONTEND=noninteractive eatmydata \ -apt install -y --no-install-recommends \ +RUN export DEBIAN_FRONTEND=noninteractive && \ +apt-get update && \ +apt-get install -y eatmydata && \ +eatmydata apt-get dist-upgrade -y && \ +eatmydata apt-get install --no-install-recommends -y \ gcc-mips64-linux-gnuabi64 \ libc6-dev-mips64-cross -- 2.34.1
[RFC PATCH 3/4] hw/intc/gic: use MxTxAttrs to divine accessing CPU
Now that MxTxAttrs encodes a CPU we should use that to figure it out.
This solves edge cases like accessing via gdbstub or qtest.
Signed-off-by: Alex Bennée
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/124
---
hw/intc/arm_gic.c | 39 ++-
1 file changed, 22 insertions(+), 17 deletions(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 492b2421ab..7feedac735 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -56,17 +56,22 @@ static const uint8_t gic_id_gicv2[] = {
0x04, 0x00, 0x00, 0x00, 0x90, 0xb4, 0x2b, 0x00, 0x0d, 0xf0, 0x05, 0xb1
};
-static inline int gic_get_current_cpu(GICState *s)
+static inline int gic_get_current_cpu(GICState *s, MemTxAttrs attrs)
{
-if (!qtest_enabled() && s->num_cpu > 1) {
-return current_cpu->cpu_index;
-}
-return 0;
+/*
+ * Something other than a CPU accessing the GIC would be a bug as
+ * would a CPU index higher than the GICState expects to be
+ * handling
+ */
+g_assert(attrs.requester_cpu == 1);
+g_assert(attrs.requester_id < s->num_cpu);
+
+return attrs.requester_id;
}
-static inline int gic_get_current_vcpu(GICState *s)
+static inline int gic_get_current_vcpu(GICState *s, MemTxAttrs attrs)
{
-return gic_get_current_cpu(s) + GIC_NCPU;
+return gic_get_current_cpu(s, attrs) + GIC_NCPU;
}
/* Return true if this GIC config has interrupt groups, which is
@@ -951,7 +956,7 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr offset,
MemTxAttrs attrs)
int cm;
int mask;
-cpu = gic_get_current_cpu(s);
+cpu = gic_get_current_cpu(s, attrs);
cm = 1 << cpu;
if (offset < 0x100) {
if (offset == 0) { /* GICD_CTLR */
@@ -1182,7 +1187,7 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
int i;
int cpu;
-cpu = gic_get_current_cpu(s);
+cpu = gic_get_current_cpu(s, attrs);
if (offset < 0x100) {
if (offset == 0) {
if (s->security_extn && !attrs.secure) {
@@ -1476,7 +1481,7 @@ static void gic_dist_writel(void *opaque, hwaddr offset,
int mask;
int target_cpu;
-cpu = gic_get_current_cpu(s);
+cpu = gic_get_current_cpu(s, attrs);
irq = value & 0xf;
switch ((value >> 24) & 3) {
case 0:
@@ -1780,7 +1785,7 @@ static MemTxResult gic_thiscpu_read(void *opaque, hwaddr
addr, uint64_t *data,
unsigned size, MemTxAttrs attrs)
{
GICState *s = (GICState *)opaque;
-return gic_cpu_read(s, gic_get_current_cpu(s), addr, data, attrs);
+return gic_cpu_read(s, gic_get_current_cpu(s, attrs), addr, data, attrs);
}
static MemTxResult gic_thiscpu_write(void *opaque, hwaddr addr,
@@ -1788,7 +1793,7 @@ static MemTxResult gic_thiscpu_write(void *opaque, hwaddr
addr,
MemTxAttrs attrs)
{
GICState *s = (GICState *)opaque;
-return gic_cpu_write(s, gic_get_current_cpu(s), addr, value, attrs);
+return gic_cpu_write(s, gic_get_current_cpu(s, attrs), addr, value, attrs);
}
/* Wrappers to read/write the GIC CPU interface for a specific CPU.
@@ -1818,7 +1823,7 @@ static MemTxResult gic_thisvcpu_read(void *opaque, hwaddr
addr, uint64_t *data,
{
GICState *s = (GICState *)opaque;
-return gic_cpu_read(s, gic_get_current_vcpu(s), addr, data, attrs);
+return gic_cpu_read(s, gic_get_current_vcpu(s, attrs), addr, data, attrs);
}
static MemTxResult gic_thisvcpu_write(void *opaque, hwaddr addr,
@@ -1827,7 +1832,7 @@ static MemTxResult gic_thisvcpu_write(void *opaque,
hwaddr addr,
{
GICState *s = (GICState *)opaque;
-return gic_cpu_write(s, gic_get_current_vcpu(s), addr, value, attrs);
+return gic_cpu_write(s, gic_get_current_vcpu(s, attrs), addr, value,
attrs);
}
static uint32_t gic_compute_eisr(GICState *s, int cpu, int lr_start)
@@ -1860,7 +1865,7 @@ static uint32_t gic_compute_elrsr(GICState *s, int cpu,
int lr_start)
static void gic_vmcr_write(GICState *s, uint32_t value, MemTxAttrs attrs)
{
-int vcpu = gic_get_current_vcpu(s);
+int vcpu = gic_get_current_vcpu(s, attrs);
uint32_t ctlr;
uint32_t abpr;
uint32_t bpr;
@@ -1995,7 +2000,7 @@ static MemTxResult gic_thiscpu_hyp_read(void *opaque,
hwaddr addr, uint64_t *dat
{
GICState *s = (GICState *)opaque;
-return gic_hyp_read(s, gic_get_current_cpu(s), addr, data, attrs);
+return gic_hyp_read(s, gic_get_current_cpu(s, attrs), addr, data, attrs);
}
static MemTxResult gic_thiscpu_hyp_write(void *opaque, hwaddr addr,
@@ -2004,7 +2009,7 @@ static MemTxResult gic_thiscpu_hyp_write(void *opaque,
hwaddr addr,
{
GICState *s = (GICState *)opaque;
-return gic_hyp_write(s, gic_get_current_cpu(s), addr, value, attrs);
+return gic_hyp_write(s, gic_get_current_cpu(s, attrs), addr, value, attrs);
}
static MemTxResult gic_do_hyp_read(void *opaque, hwaddr addr, uint64_t *data,
--
2.34.1
[PATCH v2 30/30] tests/docker: remove the Debian base images
We no longer use these in any of our images. Clean-up the remaining
comments and documentation that reference them and remove from the
build.
Signed-off-by: Alex Bennée
Reviewed-by: Thomas Huth
Message-Id: <20220826172128.353798-25-alex.ben...@linaro.org>
---
docs/devel/testing.rst | 2 +-
.gitlab-ci.d/container-core.yml | 5
.gitlab-ci.d/containers.yml | 5
tests/docker/Makefile.include| 8 +
tests/docker/dockerfiles/debian10.docker | 38
tests/docker/dockerfiles/debian11.docker | 18 ---
6 files changed, 2 insertions(+), 74 deletions(-)
delete mode 100644 tests/docker/dockerfiles/debian10.docker
delete mode 100644 tests/docker/dockerfiles/debian11.docker
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index f35f117d95..aea5b42356 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -375,7 +375,7 @@ locally by using the ``NOCACHE`` build option:
.. code::
- make docker-image-debian10 NOCACHE=1
+ make docker-image-debian-arm64-cross NOCACHE=1
Images
~~
diff --git a/.gitlab-ci.d/container-core.yml b/.gitlab-ci.d/container-core.yml
index e8dd1f476a..08f8450fa1 100644
--- a/.gitlab-ci.d/container-core.yml
+++ b/.gitlab-ci.d/container-core.yml
@@ -10,8 +10,3 @@ amd64-fedora-container:
extends: .container_job_template
variables:
NAME: fedora
-
-amd64-debian10-container:
- extends: .container_job_template
- variables:
-NAME: debian10
diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml
index be34cbc7ba..96d2a3b58b 100644
--- a/.gitlab-ci.d/containers.yml
+++ b/.gitlab-ci.d/containers.yml
@@ -7,11 +7,6 @@ amd64-alpine-container:
variables:
NAME: alpine
-amd64-debian11-container:
- extends: .container_job_template
- variables:
-NAME: debian11
-
amd64-debian-container:
extends: .container_job_template
stage: containers
diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index a3174346da..270e99786e 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -69,9 +69,7 @@ docker-binfmt-image-debian-%:
$(DOCKER_FILES_DIR)/debian-bootstrap.docker
{ echo "You will need to build $(EXECUTABLE)"; exit
1;},\
"CHECK", "debian-$* exists"))
-# Enforce dependencies for composite images
-# we don't run tests on intermediate images (used as base by another image)
-DOCKER_PARTIAL_IMAGES := debian10 debian11
+# Special case cross-compiling x86_64 on non-x86_64 systems.
ifeq ($(HOST_ARCH),x86_64)
DOCKER_PARTIAL_IMAGES += debian-amd64-cross
else
@@ -89,10 +87,6 @@ endif
# The native build should never use the registry
docker-image-debian-native: DOCKER_REGISTRY=
-# base images should not add a local user
-docker-image-debian10: NOUSER=1
-docker-image-debian11: NOUSER=1
-
# alpine has no adduser
docker-image-alpine: NOUSER=1
diff --git a/tests/docker/dockerfiles/debian10.docker
b/tests/docker/dockerfiles/debian10.docker
deleted file mode 100644
index 03be923066..00
--- a/tests/docker/dockerfiles/debian10.docker
+++ /dev/null
@@ -1,38 +0,0 @@
-#
-# Docker multiarch cross-compiler target
-#
-# This docker target is builds on Debian cross compiler targets to build distro
-# with a selection of cross compilers for building test binaries.
-#
-# On its own you can't build much but the docker-foo-cross targets
-# build on top of the base debian image.
-#
-FROM docker.io/library/debian:buster-slim
-
-# Duplicate deb line as deb-src
-RUN cat /etc/apt/sources.list | sed "s/^deb\ /deb-src /" >>
/etc/apt/sources.list
-
-# Install common build utilities
-RUN apt update && \
-DEBIAN_FRONTEND=noninteractive apt install -yy eatmydata && \
-DEBIAN_FRONTEND=noninteractive eatmydata \
-apt install -y --no-install-recommends \
-bc \
-build-essential \
-ca-certificates \
-ccache \
-clang \
-dbus \
-gdb-multiarch \
-gettext \
-git \
-libffi-dev \
-libncurses5-dev \
-ninja-build \
-pkg-config \
-psmisc \
-python3 \
-python3-sphinx \
-python3-sphinx-rtd-theme \
-python3-venv \
-$(apt-get -s build-dep --arch-only qemu | egrep ^Inst | fgrep '[all]'
| cut -d\ -f2)
diff --git a/tests/docker/dockerfiles/debian11.docker
b/tests/docker/dockerfiles/debian11.docker
deleted file mode 100644
index febf884f8f..00
--- a/tests/docker/dockerfiles/debian11.docker
+++ /dev/null
@@ -1,18 +0,0 @@
-#
-# Docker multiarch cross-compiler target
-#
-# This docker target uses the current development version of Debian as
-# a base for cross compilers for building test binaries. We won't
-# attempt to build QEMU on it yet given it is still in development.
-#
-# On its own you can't build much but the docker-foo-cross targets
-# build on top of the base debian image.
-#
-FROM
[PATCH v2 24/30] tests/lcitool: bump to latest version
We need this to be able to cleanly build the x86 cross images. There are a few minor updates triggered by lcitool-refresh including adding "libslirp" to the freebsd vars and opensuse-leap which will help when we finally drop the slirp submodule from QEMU. Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20220826172128.353798-19-alex.ben...@linaro.org> --- v2 - expand commit re:libslirp --- .gitlab-ci.d/cirrus/freebsd-12.vars | 2 +- .gitlab-ci.d/cirrus/freebsd-13.vars | 2 +- tests/docker/dockerfiles/opensuse-leap.docker | 3 ++- tests/docker/dockerfiles/ubuntu2004.docker| 2 +- tests/lcitool/libvirt-ci | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.gitlab-ci.d/cirrus/freebsd-12.vars b/.gitlab-ci.d/cirrus/freebsd-12.vars index 8fa5a320e9..1a5959810f 100644 --- a/.gitlab-ci.d/cirrus/freebsd-12.vars +++ b/.gitlab-ci.d/cirrus/freebsd-12.vars @@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake' NINJA='/usr/local/bin/ninja' PACKAGING_COMMAND='pkg' PIP3='/usr/local/bin/pip-3.8' -PKGS='alsa-lib bash bzip2 ca_root_nss capstone4 ccache cdrkit-genisoimage cmocka ctags curl cyrus-sasl dbus diffutils dtc fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs libspice-server libssh libtasn1 llvm lzo2 meson ncurses nettle ninja opencv perl5 pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy spice-protocol tesseract texinfo usbredir virglrenderer vte3 zstd' +PKGS='alsa-lib bash bzip2 ca_root_nss capstone4 ccache cdrkit-genisoimage cmocka ctags curl cyrus-sasl dbus diffutils dtc fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs libslirp libspice-server libssh libtasn1 llvm lzo2 meson ncurses nettle ninja opencv perl5 pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy spice-protocol tesseract texinfo usbredir virglrenderer vte3 zstd' PYPI_PKGS='' PYTHON='/usr/local/bin/python3' diff --git a/.gitlab-ci.d/cirrus/freebsd-13.vars b/.gitlab-ci.d/cirrus/freebsd-13.vars index 8ed7e33a77..5e5aafd7e5 100644 --- a/.gitlab-ci.d/cirrus/freebsd-13.vars +++ b/.gitlab-ci.d/cirrus/freebsd-13.vars @@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake' NINJA='/usr/local/bin/ninja' PACKAGING_COMMAND='pkg' PIP3='/usr/local/bin/pip-3.8' -PKGS='alsa-lib bash bzip2 ca_root_nss capstone4 ccache cdrkit-genisoimage cmocka ctags curl cyrus-sasl dbus diffutils dtc fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs libspice-server libssh libtasn1 llvm lzo2 meson ncurses nettle ninja opencv perl5 pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy spice-protocol tesseract texinfo usbredir virglrenderer vte3 zstd' +PKGS='alsa-lib bash bzip2 ca_root_nss capstone4 ccache cdrkit-genisoimage cmocka ctags curl cyrus-sasl dbus diffutils dtc fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs libslirp libspice-server libssh libtasn1 llvm lzo2 meson ncurses nettle ninja opencv perl5 pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy spice-protocol tesseract texinfo usbredir virglrenderer vte3 zstd' PYPI_PKGS='' PYTHON='/usr/local/bin/python3' diff --git a/tests/docker/dockerfiles/opensuse-leap.docker b/tests/docker/dockerfiles/opensuse-leap.docker index 047a435ab5..041cf9c1ff 100644 --- a/tests/docker/dockerfiles/opensuse-leap.docker +++ b/tests/docker/dockerfiles/opensuse-leap.docker @@ -66,6 +66,7 @@ RUN zypper update -y && \ librbd-devel \ libseccomp-devel \ libselinux-devel \ + libslirp-devel \ libspice-server-devel \ libssh-devel \ libtasn1-devel \ @@ -127,7 +128,7 @@ RUN zypper update -y && \ ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/g++ && \ ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/gcc -RUN pip3 install meson==0.56.0 +RUN /usr/bin/pip3 install meson==0.56.0 ENV LANG "en_US.UTF-8" ENV MAKE "/usr/bin/make" diff --git a/tests/docker/dockerfiles/ubuntu2004.docker b/tests/docker/dockerfiles/ubuntu2004.docker index 99803b343b..e1f4ed7c80 100644 --- a/tests/docker/dockerfiles/ubuntu2004.docker +++ b/tests/docker/dockerfiles/ubuntu2004.docker @@ -137,7 +137,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/g++ && \ ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/gcc -RUN pip3 install meson==0.56.0 +RUN /usr/bin/pip3 install meson==0.56.0 ENV LANG "en_US.UTF-8" ENV MAKE "/usr/bin/make" diff --git
[PATCH v2 20/30] gitlab-ci: update aarch32/aarch64 custom runner jobs
The custom runner is now using 22.04 so we can drop our hacks to deal
with broken libssh and glusterfs. The provisioning scripts will be
updated in a separate commit.
Signed-off-by: Alex Bennée
Reviewed-by: Thomas Huth
Message-Id: <20220826172128.353798-15-alex.ben...@linaro.org>
---
v2
- fix extra-cflags merge conflict
- update MAINTAINERS
---
.gitlab-ci.d/custom-runners.yml | 4 +--
...4-aarch32.yml => ubuntu-22.04-aarch32.yml} | 6 ++--
...4-aarch64.yml => ubuntu-22.04-aarch64.yml} | 36 +--
MAINTAINERS | 3 +-
4 files changed, 24 insertions(+), 25 deletions(-)
rename .gitlab-ci.d/custom-runners/{ubuntu-20.04-aarch32.yml =>
ubuntu-22.04-aarch32.yml} (86%)
rename .gitlab-ci.d/custom-runners/{ubuntu-20.04-aarch64.yml =>
ubuntu-22.04-aarch64.yml} (82%)
diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
index 15aaccc481..97f99e29c2 100644
--- a/.gitlab-ci.d/custom-runners.yml
+++ b/.gitlab-ci.d/custom-runners.yml
@@ -15,6 +15,6 @@ variables:
include:
- local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml'
- - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml'
- - local: '/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch32.yml'
+ - local: '/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml'
+ - local: '/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch32.yml'
- local: '/.gitlab-ci.d/custom-runners/centos-stream-8-x86_64.yml'
diff --git a/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch32.yml
b/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch32.yml
similarity index 86%
rename from .gitlab-ci.d/custom-runners/ubuntu-20.04-aarch32.yml
rename to .gitlab-ci.d/custom-runners/ubuntu-22.04-aarch32.yml
index cbfa9cc164..1a2f9b8dbe 100644
--- a/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch32.yml
+++ b/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch32.yml
@@ -1,12 +1,12 @@
-# All ubuntu-20.04 jobs should run successfully in an environment
+# All ubuntu-22.04 jobs should run successfully in an environment
# setup by the scripts/ci/setup/qemu/build-environment.yml task
# "Install basic packages to build QEMU on Ubuntu 20.04"
-ubuntu-20.04-aarch32-all:
+ubuntu-22.04-aarch32-all:
needs: []
stage: build
tags:
- - ubuntu_20.04
+ - ubuntu_22.04
- aarch32
rules:
- if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~
/^staging/'
diff --git a/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml
b/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml
similarity index 82%
rename from .gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml
rename to .gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml
index 3f4dee4f86..ce0b18af6f 100644
--- a/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml
+++ b/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml
@@ -2,23 +2,21 @@
# setup by the scripts/ci/setup/qemu/build-environment.yml task
# "Install basic packages to build QEMU on Ubuntu 20.04"
-ubuntu-20.04-aarch64-all-linux-static:
+ubuntu-22.04-aarch64-all-linux-static:
needs: []
stage: build
tags:
- - ubuntu_20.04
+ - ubuntu_22.04
- aarch64
rules:
- if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~
/^staging/'
- if: "$AARCH64_RUNNER_AVAILABLE"
script:
- # --disable-libssh is needed because of
https://bugs.launchpad.net/qemu/+bug/1838763
- # --disable-glusterfs is needed because there's no static version of those
libs in distro supplied packages
- mkdir build
- cd build
# Disable -static-pie due to build error with system libc:
# https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/1987438
- - ../configure --enable-debug --static --disable-system --disable-glusterfs
--disable-libssh --disable-pie
+ - ../configure --enable-debug --static --disable-system --disable-pie
|| { cat config.log meson-logs/meson-log.txt; exit 1; }
- make --output-sync -j`nproc --ignore=40`
- make --output-sync -j`nproc --ignore=40` check V=1
@@ -26,11 +24,11 @@ ubuntu-20.04-aarch64-all-linux-static:
- make --output-sync -j`nproc --ignore=40` check-tcg V=1
|| { cat meson-logs/testlog.txt; exit 1; } ;
-ubuntu-20.04-aarch64-all:
+ubuntu-22.04-aarch64-all:
needs: []
stage: build
tags:
- - ubuntu_20.04
+ - ubuntu_22.04
- aarch64
rules:
- if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~
/^staging/'
@@ -42,17 +40,17 @@ ubuntu-20.04-aarch64-all:
script:
- mkdir build
- cd build
- - ../configure --disable-libssh
+ - ../configure
|| { cat config.log meson-logs/meson-log.txt; exit 1; }
- make --output-sync -j`nproc --ignore=40`
- make --output-sync -j`nproc --ignore=40` check V=1
|| { cat meson-logs/testlog.txt; exit 1; } ;
-ubuntu-20.04-aarch64-alldbg:
+ubuntu-22.04-aarch64-alldbg:
needs: []
stage: build
tags:
- - ubuntu_20.04
+ - ubuntu_22.04
- aarch64
rules:
- if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH =~
/^staging/'
@@ -60,18 +58,18 @@
[PATCH v2 26/30] tests/docker: update and flatten debian-loongarch-cross
Update to the latest stable Debian. While we are at it flatten into a single dockerfile. We really don't need the rest of the stuff from the QEMU base image just to compile test images. In this case it is a binary distribution of the toolchain anyway. Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20220826172128.353798-21-alex.ben...@linaro.org> --- v2 - use debian:11-slim for consistency with the others --- tests/docker/Makefile.include | 1 - tests/docker/dockerfiles/debian-loongarch-cross.docker | 5 - 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index a0f5109628..1d5a6f1fb4 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -128,7 +128,6 @@ docker-image-debian-nios2-cross: $(DOCKER_FILES_DIR)/debian-toolchain.docker \ $(call debian-toolchain, $@) # Specialist build images, sometimes very limited tools -docker-image-debian-loongarch-cross: docker-image-debian11 docker-image-debian-microblaze-cross: docker-image-debian10 docker-image-debian-nios2-cross: docker-image-debian10 diff --git a/tests/docker/dockerfiles/debian-loongarch-cross.docker b/tests/docker/dockerfiles/debian-loongarch-cross.docker index ca2469d2a8..df578da40d 100644 --- a/tests/docker/dockerfiles/debian-loongarch-cross.docker +++ b/tests/docker/dockerfiles/debian-loongarch-cross.docker @@ -5,7 +5,10 @@ # using a prebuilt toolchains for LoongArch64 from: # https://github.com/loongson/build-tools/releases # -FROM qemu/debian11 +FROM docker.io/library/debian:11-slim + +# Duplicate deb line as deb-src +RUN cat /etc/apt/sources.list | sed "s/^deb\ /deb-src /" >> /etc/apt/sources.list RUN apt-get update && \ DEBIAN_FRONTEND=noninteractive apt install -yy eatmydata && \ -- 2.34.1
[PATCH v2 27/30] tests/docker: update and flatten debian-hexagon-cross
Update to the latest stable Debian. While we are at it flatten into a single dockerfile as we do not some of the extraneous packages from the base image to build the toolchain. Signed-off-by: Alex Bennée Message-Id: <20220826172128.353798-22-alex.ben...@linaro.org> --- v2 - use debian11 consistently - remove "stand-alone" comments which will soon be irrelevant - reword commit as we do need some extra packages --- .../dockerfiles/debian-hexagon-cross.docker | 19 --- .../dockerfiles/debian-loongarch-cross.docker | 3 +-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/tests/docker/dockerfiles/debian-hexagon-cross.docker b/tests/docker/dockerfiles/debian-hexagon-cross.docker index d5dc299dc1..8d219bb81d 100644 --- a/tests/docker/dockerfiles/debian-hexagon-cross.docker +++ b/tests/docker/dockerfiles/debian-hexagon-cross.docker @@ -2,12 +2,10 @@ # Docker Hexagon cross-compiler target # # This docker target is used for building hexagon tests. As it also -# needs to be able to build QEMU itself in CI we include it's -# build-deps. It is also a "stand-alone" image so as not to be -# triggered by re-builds on other base images given it takes a long -# time to build. +# needs to be able to build QEMU itself in CI we include its +# build-deps. # -FROM qemu/debian10 +FROM docker.io/library/debian:11-slim # Install common build utilities RUN apt update && \ @@ -15,11 +13,18 @@ RUN apt update && \ DEBIAN_FRONTEND=noninteractive eatmydata \ apt install -y --no-install-recommends \ bison \ +ca-certificates \ +clang \ cmake \ flex \ +gcc \ lld \ +make \ +ninja-build \ +python3 \ rsync \ -wget +wget \ +xz-utils ENV TOOLCHAIN_INSTALL /usr/local ENV ROOTFS /usr/local @@ -32,7 +37,7 @@ ADD build-toolchain.sh /root/hexagon-toolchain/build-toolchain.sh RUN cd /root/hexagon-toolchain && ./build-toolchain.sh -FROM debian:buster-slim +FROM docker.io/library/debian:11-slim # Duplicate deb line as deb-src RUN cat /etc/apt/sources.list | sed "s/^deb\ /deb-src /" >> /etc/apt/sources.list # Install QEMU build deps for use in CI diff --git a/tests/docker/dockerfiles/debian-loongarch-cross.docker b/tests/docker/dockerfiles/debian-loongarch-cross.docker index df578da40d..a8e8e98909 100644 --- a/tests/docker/dockerfiles/debian-loongarch-cross.docker +++ b/tests/docker/dockerfiles/debian-loongarch-cross.docker @@ -1,8 +1,7 @@ # # Docker cross-compiler target # -# This docker target builds on the debian11 base image, -# using a prebuilt toolchains for LoongArch64 from: +# This docker target uses prebuilt toolchains for LoongArch64 from: # https://github.com/loongson/build-tools/releases # FROM docker.io/library/debian:11-slim -- 2.34.1
[PATCH v2 25/30] tests/docker: update and flatten debian-amd64-cross
Now lcitool has support for building a x86_64 cross image we can use it for this. Signed-off-by: Alex Bennée Acked-by: Thomas Huth Message-Id: <20220826172128.353798-20-alex.ben...@linaro.org> --- .gitlab-ci.d/container-cross.yml | 1 - tests/docker/Makefile.include | 1 - .../dockerfiles/debian-amd64-cross.docker | 178 -- tests/lcitool/refresh | 7 + 4 files changed, 169 insertions(+), 18 deletions(-) diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml index 091c0d8fcb..2d560e9764 100644 --- a/.gitlab-ci.d/container-cross.yml +++ b/.gitlab-ci.d/container-cross.yml @@ -7,7 +7,6 @@ alpha-debian-cross-container: amd64-debian-cross-container: extends: .container_job_template stage: containers - needs: ['amd64-debian10-container'] variables: NAME: debian-amd64-cross diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index ddcc502049..a0f5109628 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -75,7 +75,6 @@ DOCKER_PARTIAL_IMAGES := debian10 debian11 ifeq ($(HOST_ARCH),x86_64) DOCKER_PARTIAL_IMAGES += debian-amd64-cross else -docker-image-debian-amd64-cross: docker-image-debian10 DOCKER_PARTIAL_IMAGES += debian-amd64 endif diff --git a/tests/docker/dockerfiles/debian-amd64-cross.docker b/tests/docker/dockerfiles/debian-amd64-cross.docker index 870109ef6a..7d2feb7bf7 100644 --- a/tests/docker/dockerfiles/debian-amd64-cross.docker +++ b/tests/docker/dockerfiles/debian-amd64-cross.docker @@ -1,22 +1,168 @@ +# THIS FILE WAS AUTO-GENERATED # -# Docker x86_64 cross target +# $ lcitool dockerfile --layers all --cross x86_64 debian-11 qemu # -# This docker target is used on non-x86_64 machines which need the -# x86_64 cross compilers installed. -# -FROM qemu/debian10 -MAINTAINER Alex Bennée +# https://gitlab.com/libvirt/libvirt-ci + +FROM docker.io/library/debian:11-slim + +RUN export DEBIAN_FRONTEND=noninteractive && \ +apt-get update && \ +apt-get install -y eatmydata && \ +eatmydata apt-get dist-upgrade -y && \ +eatmydata apt-get install --no-install-recommends -y \ +bash \ +bc \ +bsdextrautils \ +bzip2 \ +ca-certificates \ +ccache \ +dbus \ +debianutils \ +diffutils \ +exuberant-ctags \ +findutils \ +gcovr \ +genisoimage \ +gettext \ +git \ +hostname \ +libpcre2-dev \ +libspice-protocol-dev \ +llvm \ +locales \ +make \ +meson \ +ncat \ +ninja-build \ +openssh-client \ +perl-base \ +pkgconf \ +python3 \ +python3-numpy \ +python3-opencv \ +python3-pillow \ +python3-pip \ +python3-sphinx \ +python3-sphinx-rtd-theme \ +python3-venv \ +python3-yaml \ +rpm2cpio \ +sed \ +sparse \ +tar \ +tesseract-ocr \ +tesseract-ocr-eng \ +texinfo && \ +eatmydata apt-get autoremove -y && \ +eatmydata apt-get autoclean -y && \ +sed -Ei 's,^# (en_US\.UTF-8 .*)$,\1,' /etc/locale.gen && \ +dpkg-reconfigure locales + +ENV LANG "en_US.UTF-8" +ENV MAKE "/usr/bin/make" +ENV NINJA "/usr/bin/ninja" +ENV PYTHON "/usr/bin/python3" +ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers" -# Add the foreign architecture we want and install dependencies -RUN dpkg --add-architecture amd64 -RUN apt update && \ -DEBIAN_FRONTEND=noninteractive eatmydata \ -apt install -y --no-install-recommends \ -crossbuild-essential-amd64 -RUN apt update && \ -DEBIAN_FRONTEND=noninteractive eatmydata \ -apt build-dep -yy -a amd64 --arch-only qemu +RUN export DEBIAN_FRONTEND=noninteractive && \ +dpkg --add-architecture amd64 && \ +eatmydata apt-get update && \ +eatmydata apt-get dist-upgrade -y && \ +eatmydata apt-get install --no-install-recommends -y dpkg-dev && \ +eatmydata apt-get install --no-install-recommends -y \ +g++-x86-64-linux-gnu \ +gcc-x86-64-linux-gnu \ +libaio-dev:amd64 \ +libasan5:amd64 \ +libasound2-dev:amd64 \ +libattr1-dev:amd64 \ +libbpf-dev:amd64 \ +libbrlapi-dev:amd64 \ +libbz2-dev:amd64 \ +libc6-dev:amd64 \ +libcacard-dev:amd64 \ +libcap-ng-dev:amd64 \ +libcapstone-dev:amd64 \ +libcmocka-dev:amd64 \ +libcurl4-gnutls-dev:amd64 \ +libdaxctl-dev:amd64 \ +libdrm-dev:amd64 \ +libepoxy-dev:amd64 \ +libfdt-dev:amd64 \ +libffi-dev:amd64 \ +
[PATCH v2 28/30] tests/docker: update and flatten debian-toolchain
Update to the latest stable Debian. While we are at it flatten into a single dockerfile as we do not need anything from the base image to build the toolchain. This is used to build both the nios and microblaze toolchains. Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20220826172128.353798-23-alex.ben...@linaro.org> --- tests/docker/Makefile.include| 4 tests/docker/dockerfiles/debian-toolchain.docker | 5 +++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 1d5a6f1fb4..a3174346da 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -127,10 +127,6 @@ docker-image-debian-nios2-cross: $(DOCKER_FILES_DIR)/debian-toolchain.docker \ $(DOCKER_FILES_DIR)/debian-nios2-cross.d/build-toolchain.sh $(call debian-toolchain, $@) -# Specialist build images, sometimes very limited tools -docker-image-debian-microblaze-cross: docker-image-debian10 -docker-image-debian-nios2-cross: docker-image-debian10 - # These images may be good enough for building tests but not for test builds DOCKER_PARTIAL_IMAGES += debian-alpha-cross DOCKER_PARTIAL_IMAGES += debian-powerpc-test-cross diff --git a/tests/docker/dockerfiles/debian-toolchain.docker b/tests/docker/dockerfiles/debian-toolchain.docker index 738d808aa6..c723377495 100644 --- a/tests/docker/dockerfiles/debian-toolchain.docker +++ b/tests/docker/dockerfiles/debian-toolchain.docker @@ -4,7 +4,7 @@ # This dockerfile is used for building a cross-compiler toolchain. # The script for building the toolchain is supplied via extra-files. # -FROM qemu/debian10 +FROM docker.io/library/debian:bullseye-slim # Install build utilities for building gcc and glibc. # ??? The build-dep isn't working, missing a number of @@ -15,6 +15,7 @@ RUN apt update && \ DEBIAN_FRONTEND=noninteractive eatmydata \ apt install -y --no-install-recommends \ bison \ +ca-certificates \ flex \ gawk \ libmpc-dev \ @@ -32,5 +33,5 @@ RUN cd /root && ./build-toolchain.sh # Throw away the extra toolchain build deps, the downloaded source, # and the build trees by restoring the original debian10 image, # then copying the built toolchain from stage 0. -FROM qemu/debian10 +FROM docker.io/library/debian:bullseye-slim COPY --from=0 /usr/local /usr/local -- 2.34.1
[PATCH v2 19/30] gitlab-ci/custom-runners: Disable -static-pie for ubuntu-20.04-aarch64
From: Richard Henderson
The project has reached the magic size at which we see
/usr/aarch64-linux-gnu/lib/libc.a(init-first.o): in function
`__libc_init_first':
(.text+0x10): relocation truncated to fit: R_AARCH64_LD64_GOTPAGE_LO15 against \
symbol `__environ' defined in .bss section in
/usr/aarch64-linux-gnu/lib/libc.a(environ.o)
/usr/bin/ld: (.text+0x10): warning: too many GOT entries for -fpic, please
recompile with -fPIC
The bug has been reported upstream, but in the meantime there is
nothing we can do except build a non-pie executable.
Signed-off-by: Richard Henderson
Signed-off-by: Alex Bennée
Message-Id: <20220823210329.1969895-1-richard.hender...@linaro.org>
Message-Id: <20220826172128.353798-14-alex.ben...@linaro.org>
---
v2
- rm --extra-cflags workaround (now in configure)
---
.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml
b/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml
index 3d878914e7..3f4dee4f86 100644
--- a/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml
+++ b/.gitlab-ci.d/custom-runners/ubuntu-20.04-aarch64.yml
@@ -16,7 +16,9 @@ ubuntu-20.04-aarch64-all-linux-static:
# --disable-glusterfs is needed because there's no static version of those
libs in distro supplied packages
- mkdir build
- cd build
- - ../configure --enable-debug --static --disable-system --disable-glusterfs
--disable-libssh
+ # Disable -static-pie due to build error with system libc:
+ # https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/1987438
+ - ../configure --enable-debug --static --disable-system --disable-glusterfs
--disable-libssh --disable-pie
|| { cat config.log meson-logs/meson-log.txt; exit 1; }
- make --output-sync -j`nproc --ignore=40`
- make --output-sync -j`nproc --ignore=40` check V=1
--
2.34.1
[PATCH v2 29/30] tests/docker: remove FROM qemu/ support from docker.py
We want to migrate from docker.py to building our images directly with
docker/podman. Before we get there we need to make sure we don't
re-introduce our layered builds so bug out if we see FROM qemu/ in a
Dockerfile.
Signed-off-by: Alex Bennée
Acked-by: Thomas Huth
Message-Id: <20220826172128.353798-24-alex.ben...@linaro.org>
---
tests/docker/docker.py | 38 ++
1 file changed, 10 insertions(+), 28 deletions(-)
diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index d0af2861b8..3a1ed7cb18 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -205,22 +205,17 @@ def _read_qemu_dockerfile(img_name):
return _read_dockerfile(df)
-def _dockerfile_preprocess(df):
-out = ""
+def _dockerfile_verify_flat(df):
+"Verify we do not include other qemu/ layers"
for l in df.splitlines():
if len(l.strip()) == 0 or l.startswith("#"):
continue
from_pref = "FROM qemu/"
if l.startswith(from_pref):
-# TODO: Alternatively we could replace this line with "FROM $ID"
-# where $ID is the image's hex id obtained with
-#$ docker images $IMAGE --format="{{.Id}}"
-# but unfortunately that's not supported by RHEL 7.
-inlining = _read_qemu_dockerfile(l[len(from_pref):])
-out += _dockerfile_preprocess(inlining)
-continue
-out += l + "\n"
-return out
+print("We no longer support multiple QEMU layers.")
+print("Dockerfiles should be flat, ideally created by lcitool")
+return False
+return True
class Docker(object):
@@ -309,23 +304,10 @@ def build_image(self, tag, docker_dir, dockerfile,
if argv is None:
argv = []
-# pre-calculate the docker checksum before any
-# substitutions we make for caching
-checksum = _text_checksum(_dockerfile_preprocess(dockerfile))
+if not _dockerfile_verify_flat(dockerfile):
+return -1
-if registry is not None:
-sources = re.findall("FROM qemu\/(.*)", dockerfile)
-# Fetch any cache layers we can, may fail
-for s in sources:
-pull_args = ["pull", "%s/qemu/%s" % (registry, s)]
-if self._do(pull_args, quiet=quiet) != 0:
-registry = None
-break
-# Make substitutions
-if registry is not None:
-dockerfile = dockerfile.replace("FROM qemu/",
-"FROM %s/qemu/" %
-(registry))
+checksum = _text_checksum(dockerfile)
tmp_df = tempfile.NamedTemporaryFile(mode="w+t",
encoding='utf-8',
@@ -371,7 +353,7 @@ def image_matches_dockerfile(self, tag, dockerfile):
checksum = self.get_image_dockerfile_checksum(tag)
except Exception:
return False
-return checksum == _text_checksum(_dockerfile_preprocess(dockerfile))
+return checksum == _text_checksum(dockerfile)
def run(self, cmd, keep, quiet, as_user=False):
label = uuid.uuid4().hex
--
2.34.1
[PATCH v2 21/30] Deprecate 32 bit big-endian MIPS
It's becoming harder to maintain a cross-compiler to test this host
architecture as the old stable Debian 10 ("Buster") moved into LTS
which supports fewer architectures. For now:
- mark it's deprecation in the docs
- downgrade the containers to build TCG tests only
- drop the cross builds from our CI
Users with an appropriate toolchain and user-space can still take
their chances building it.
Signed-off-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Huacai Chen
Message-Id: <20220826172128.353798-16-alex.ben...@linaro.org>
---
v2
- explicit little endian instead of LE
- s/A/As/
- restore mips to dockerfile
---
docs/about/build-platforms.rst| 2 +-
docs/about/deprecated.rst | 13 +++
.gitlab-ci.d/container-cross.yml | 1 -
.gitlab-ci.d/crossbuilds.yml | 14 ---
tests/docker/Makefile.include | 5 +--
.../dockerfiles/debian-mips-cross.docker | 38 +--
6 files changed, 26 insertions(+), 47 deletions(-)
diff --git a/docs/about/build-platforms.rst b/docs/about/build-platforms.rst
index a2fee53248..1c1e7b9e11 100644
--- a/docs/about/build-platforms.rst
+++ b/docs/about/build-platforms.rst
@@ -41,7 +41,7 @@ Those hosts are officially supported, with various
accelerators:
- Accelerators
* - Arm
- kvm (64 bit only), tcg, xen
- * - MIPS
+ * - MIPS (little endian only)
- kvm, tcg
* - PPC
- kvm, tcg
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index c75a25daad..0d1fd4469b 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -213,6 +213,19 @@ MIPS ``Trap-and-Emul`` KVM support (since 6.0)
The MIPS ``Trap-and-Emul`` KVM host and guest support has been removed
from Linux upstream kernel, declare it deprecated.
+Host Architectures
+--
+
+BE MIPS (since 7.2)
+'''
+
+As Debian 10 ("Buster") moved into LTS the big endian 32 bit version of
+MIPS moved out of support making it hard to maintain our
+cross-compilation CI tests of the architecture. As we no longer have
+CI coverage support may bitrot away before the deprecation process
+completes. The little endian variants of MIPS (both 32 and 64 bit) are
+still a supported host architecture.
+
QEMU API (QAPI) events
--
diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml
index 611c6c0b39..95d57e1c5d 100644
--- a/.gitlab-ci.d/container-cross.yml
+++ b/.gitlab-ci.d/container-cross.yml
@@ -89,7 +89,6 @@ mips64el-debian-cross-container:
mips-debian-cross-container:
extends: .container_job_template
stage: containers
- needs: ['amd64-debian10-container']
variables:
NAME: debian-mips-cross
diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index 4a5fb6ea2a..c4cd96433d 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -70,20 +70,6 @@ cross-i386-tci:
EXTRA_CONFIGURE_OPTS:
--target-list=i386-softmmu,i386-linux-user,aarch64-softmmu,aarch64-linux-user,ppc-softmmu,ppc-linux-user
MAKE_CHECK_ARGS: check check-tcg
-cross-mips-system:
- extends: .cross_system_build_job
- needs:
-job: mips-debian-cross-container
- variables:
-IMAGE: debian-mips-cross
-
-cross-mips-user:
- extends: .cross_user_build_job
- needs:
-job: mips-debian-cross-container
- variables:
-IMAGE: debian-mips-cross
-
cross-mipsel-system:
extends: .cross_system_build_job
needs:
diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index c3375f89c5..b1bf56434f 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -81,14 +81,12 @@ endif
# For non-x86 hosts not all cross-compilers have been packaged
ifneq ($(HOST_ARCH),x86_64)
-DOCKER_PARTIAL_IMAGES += debian-mips-cross debian-mipsel-cross
debian-mips64el-cross
+DOCKER_PARTIAL_IMAGES += debian-mipsel-cross debian-mips64el-cross
DOCKER_PARTIAL_IMAGES += debian-ppc64el-cross
DOCKER_PARTIAL_IMAGES += debian-s390x-cross
DOCKER_PARTIAL_IMAGES += fedora
endif
-docker-image-debian-mips-cross: docker-image-debian10
-
# The native build should never use the registry
docker-image-debian-native: DOCKER_REGISTRY=
@@ -144,6 +142,7 @@ DOCKER_PARTIAL_IMAGES += debian-hppa-cross
DOCKER_PARTIAL_IMAGES += debian-loongarch-cross
DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross
DOCKER_PARTIAL_IMAGES += debian-microblaze-cross
+DOCKER_PARTIAL_IMAGES += debian-mips-cross
DOCKER_PARTIAL_IMAGES += debian-nios2-cross
DOCKER_PARTIAL_IMAGES += debian-riscv64-test-cross
DOCKER_PARTIAL_IMAGES += debian-sh4-cross debian-sparc64-cross
diff --git a/tests/docker/dockerfiles/debian-mips-cross.docker
b/tests/docker/dockerfiles/debian-mips-cross.docker
index 26c154014d..7b55f0f3b2 100644
--- a/tests/docker/dockerfiles/debian-mips-cross.docker
+++ b/tests/docker/dockerfiles/debian-mips-cross.docker
@@ -1,32
[PATCH v2 17/30] tests/vm: Remove obsolete Fedora VM test
From: Thomas Huth
It's still based on Fedora 30 - which is not supported anymore by QEMU
since years. Seems like nobody is using (and refreshing) this, and it's
easier to test this via a container anyway, so let's remove this now.
Signed-off-by: Thomas Huth
Message-Id: <20220822175317.190551-1-th...@redhat.com>
Signed-off-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20220826172128.353798-13-alex.ben...@linaro.org>
---
tests/vm/Makefile.include | 3 +-
tests/vm/fedora | 190 --
2 files changed, 1 insertion(+), 192 deletions(-)
delete mode 100755 tests/vm/fedora
diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index 8d2a164552..2cc2203d09 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -15,7 +15,7 @@ endif
EFI_AARCH64 = $(wildcard $(BUILD_DIR)/pc-bios/edk2-aarch64-code.fd)
-X86_IMAGES := freebsd netbsd openbsd fedora haiku.x86_64
+X86_IMAGES := freebsd netbsd openbsd haiku.x86_64
ifneq ($(GENISOIMAGE),)
X86_IMAGES += centos
ifneq ($(EFI_AARCH64),)
@@ -45,7 +45,6 @@ vm-help vm-test:
@echo " vm-build-freebsd- Build QEMU in FreeBSD VM"
@echo " vm-build-netbsd - Build QEMU in NetBSD VM"
@echo " vm-build-openbsd- Build QEMU in OpenBSD VM"
- @echo " vm-build-fedora - Build QEMU in Fedora VM"
ifneq ($(GENISOIMAGE),)
@echo " vm-build-centos - Build QEMU in CentOS VM,
with Docker"
ifneq ($(EFI_AARCH64),)
diff --git a/tests/vm/fedora b/tests/vm/fedora
deleted file mode 100755
index 12eca919a0..00
--- a/tests/vm/fedora
+++ /dev/null
@@ -1,190 +0,0 @@
-#!/usr/bin/env python3
-#
-# Fedora VM image
-#
-# Copyright 2019 Red Hat Inc.
-#
-# Authors:
-# Gerd Hoffmann
-#
-# This code is licensed under the GPL version 2 or later. See
-# the COPYING file in the top-level directory.
-#
-
-import os
-import re
-import sys
-import time
-import socket
-import subprocess
-import basevm
-
-class FedoraVM(basevm.BaseVM):
-name = "fedora"
-arch = "x86_64"
-
-base =
"https://archives.fedoraproject.org/pub/archive/fedora/linux/releases/30/;
-link = base + "Server/x86_64/iso/Fedora-Server-netinst-x86_64-30-1.2.iso"
-repo = base + "Server/x86_64/os/"
-full = base + "Everything/x86_64/os/"
-csum = "5e4eac4566d8c572bfb3bcf54b7d6c82006ec3c6c882a2c9235c6d3494d7b100"
-size = "20G"
-pkgs = [
-# tools
-'git-core',
-'gcc', 'binutils', 'make', 'ninja-build',
-
-# perl
-'perl',
-
-# libs: usb
-'"pkgconfig(libusb-1.0)"',
-'"pkgconfig(libusbredirparser-0.5)"',
-
-# libs: crypto
-'"pkgconfig(gnutls)"',
-
-# libs: ui
-'"pkgconfig(sdl2)"',
-'"pkgconfig(gtk+-3.0)"',
-'"pkgconfig(ncursesw)"',
-
-# libs: audio
-'"pkgconfig(libpulse)"',
-'"pkgconfig(alsa)"',
-
-# libs: migration
-'"pkgconfig(libzstd)"',
-]
-
-BUILD_SCRIPT = """
-set -e;
-rm -rf /home/qemu/qemu-test.*
-cd $(mktemp -d /home/qemu/qemu-test.XX);
-mkdir src build; cd src;
-tar -xf /dev/vdb;
-cd ../build
-../src/configure --python=python3 {configure_opts};
-gmake --output-sync -j{jobs} {target} {verbose};
-"""
-
-def build_image(self, img):
-self.print_step("Downloading install iso")
-cimg = self._download_with_cache(self.link, sha256sum=self.csum)
-img_tmp = img + ".tmp"
-iso = img + ".install.iso"
-
-self.print_step("Preparing iso and disk image")
-subprocess.check_call(["cp", "-f", cimg, iso])
-self.exec_qemu_img("create", "-f", "qcow2", img_tmp, self.size)
-self.print_step("Booting installer")
-self.boot(img_tmp, extra_args = [
-"-machine", "graphics=off",
-"-device", "VGA",
-"-cdrom", iso
-])
-self.console_init(300)
-self.console_wait("installation process.")
-time.sleep(0.3)
-self.console_send("\t")
-time.sleep(0.3)
-self.console_send(" console=ttyS0")
-proxy = os.environ.get("http_proxy")
-if not proxy is None:
-self.console_send(" proxy=%s" % proxy)
-self.console_send(" inst.proxy=%s" % proxy)
-self.console_send(" inst.repo=%s" % self.repo)
-self.console_send("\n")
-
-self.console_wait_send("2) Use text mode", "2\n")
-
-self.console_wait_send("5) [!] Installation Dest", "5\n")
-self.console_wait_send("1) [x]", "c\n")
-self.console_wait_send("2) [ ] Use All Space", "2\n")
-self.console_wait_send("2) [x] Use All Space", "c\n")
-self.console_wait_send("1) [ ] Standard Part", "1\n")
-self.console_wait_send("1) [x] Standard Part", "c\n")
-
-
[PATCH v2 15/30] tests/docker: remove tricore qemu/debian10 dependency
We missed removing this dependency when we flattened the build.
Fixes: 39ce923732 (gitlab: enable a very minimal build with the tricore
container)
Signed-off-by: Alex Bennée
Fixes: 39ce923732 ("gitlab: enable a very minimal build with the tricore
container")
Message-Id: <20220826172128.353798-11-alex.ben...@linaro.org>
---
.gitlab-ci.d/container-cross.yml | 1 -
tests/docker/Makefile.include| 1 -
2 files changed, 2 deletions(-)
diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml
index 67bbf19a27..611c6c0b39 100644
--- a/.gitlab-ci.d/container-cross.yml
+++ b/.gitlab-ci.d/container-cross.yml
@@ -148,7 +148,6 @@ sparc64-debian-cross-container:
tricore-debian-cross-container:
extends: .container_job_template
stage: containers
- needs: ['amd64-debian10-container']
variables:
NAME: debian-tricore-cross
diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index e034eca3af..5c9398bbc9 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -132,7 +132,6 @@ docker-image-debian-nios2-cross:
$(DOCKER_FILES_DIR)/debian-toolchain.docker \
$(call debian-toolchain, $@)
# Specialist build images, sometimes very limited tools
-docker-image-debian-tricore-cross: docker-image-debian10
docker-image-debian-all-test-cross: docker-image-debian10
docker-image-debian-loongarch-cross: docker-image-debian11
docker-image-debian-microblaze-cross: docker-image-debian10
--
2.34.1
[PATCH v2 14/30] tests/docker: flatten debian-powerpc-test-cross
Flatten into a single dockerfile. We really don't need the rest of the stuff from the QEMU base image just to compile test images. Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20220826172128.353798-10-alex.ben...@linaro.org> --- .gitlab-ci.d/container-cross.yml | 1 - tests/docker/Makefile.include| 1 - .../dockerfiles/debian-powerpc-test-cross.docker | 12 +++- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml index db0ea15d0d..67bbf19a27 100644 --- a/.gitlab-ci.d/container-cross.yml +++ b/.gitlab-ci.d/container-cross.yml @@ -102,7 +102,6 @@ mipsel-debian-cross-container: powerpc-test-cross-container: extends: .container_job_template stage: containers - needs: ['amd64-debian11-container'] variables: NAME: debian-powerpc-test-cross diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 8828b6b8fa..e034eca3af 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -137,7 +137,6 @@ docker-image-debian-all-test-cross: docker-image-debian10 docker-image-debian-loongarch-cross: docker-image-debian11 docker-image-debian-microblaze-cross: docker-image-debian10 docker-image-debian-nios2-cross: docker-image-debian10 -docker-image-debian-powerpc-test-cross: docker-image-debian11 docker-image-debian-riscv64-test-cross: docker-image-debian11 # These images may be good enough for building tests but not for test builds diff --git a/tests/docker/dockerfiles/debian-powerpc-test-cross.docker b/tests/docker/dockerfiles/debian-powerpc-test-cross.docker index 36b336f709..d6b2909cc4 100644 --- a/tests/docker/dockerfiles/debian-powerpc-test-cross.docker +++ b/tests/docker/dockerfiles/debian-powerpc-test-cross.docker @@ -1,13 +1,15 @@ # # Docker powerpc/ppc64/ppc64le cross-compiler target # -# This docker target builds on the debian Bullseye base image. +# This docker target builds on the Debian Bullseye base image. # -FROM qemu/debian11 +FROM docker.io/library/debian:11-slim -RUN apt update && \ -DEBIAN_FRONTEND=noninteractive eatmydata \ -apt install -y --no-install-recommends \ +RUN export DEBIAN_FRONTEND=noninteractive && \ +apt-get update && \ +apt-get install -y eatmydata && \ +eatmydata apt-get dist-upgrade -y && \ +eatmydata apt-get install --no-install-recommends -y \ gcc-powerpc-linux-gnu \ libc6-dev-powerpc-cross \ gcc-10-powerpc64-linux-gnu \ -- 2.34.1
[PATCH v2 13/30] tests/docker: update and flatten debian-sparc64-cross
Update to the latest stable Debian. While we are at it flatten into a single dockerfile. We really don't need the rest of the stuff from the QEMU base image just to compile test images. Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20220826172128.353798-9-alex.ben...@linaro.org> --- .gitlab-ci.d/container-cross.yml | 1 - tests/docker/Makefile.include| 1 - tests/docker/dockerfiles/debian-sparc64-cross.docker | 12 +++- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml index 8a611fc824..db0ea15d0d 100644 --- a/.gitlab-ci.d/container-cross.yml +++ b/.gitlab-ci.d/container-cross.yml @@ -143,7 +143,6 @@ sh4-debian-cross-container: sparc64-debian-cross-container: extends: .container_job_template stage: containers - needs: ['amd64-debian10-container'] variables: NAME: debian-sparc64-cross diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 37c4ea913f..8828b6b8fa 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -89,7 +89,6 @@ DOCKER_PARTIAL_IMAGES += fedora endif docker-image-debian-mips-cross: docker-image-debian10 -docker-image-debian-sparc64-cross: docker-image-debian10 # The native build should never use the registry docker-image-debian-native: DOCKER_REGISTRY= diff --git a/tests/docker/dockerfiles/debian-sparc64-cross.docker b/tests/docker/dockerfiles/debian-sparc64-cross.docker index f4bb9b561c..8d3d306bc1 100644 --- a/tests/docker/dockerfiles/debian-sparc64-cross.docker +++ b/tests/docker/dockerfiles/debian-sparc64-cross.docker @@ -1,12 +1,14 @@ # # Docker cross-compiler target # -# This docker target builds on the debian Buster base image. +# This docker target builds on the Debian Bullseye base image. # -FROM qemu/debian10 +FROM docker.io/library/debian:11-slim -RUN apt update && \ -DEBIAN_FRONTEND=noninteractive eatmydata \ -apt install -y --no-install-recommends \ +RUN export DEBIAN_FRONTEND=noninteractive && \ +apt-get update && \ +apt-get install -y eatmydata && \ +eatmydata apt-get dist-upgrade -y && \ +eatmydata apt-get install --no-install-recommends -y \ gcc-sparc64-linux-gnu \ libc6-dev-sparc64-cross -- 2.34.1
[PATCH v2 05/30] tests/avocado: add explicit timeout for ppc64le TCG tests
We don't want to rely on the soon to be reduced default time. These tests are still slow for something we want to run in CI though. Signed-off-by: Alex Bennée --- tests/avocado/boot_linux.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py index 4f07c27ac6..b7522ad3a1 100644 --- a/tests/avocado/boot_linux.py +++ b/tests/avocado/boot_linux.py @@ -115,6 +115,8 @@ class BootLinuxPPC64(LinuxTest): :avocado: tags=arch:ppc64 """ +timeout = 180 + def test_pseries_tcg(self): """ :avocado: tags=machine:pseries -- 2.34.1
[PATCH v2 12/30] tests/docker: update and flatten debian-sh4-cross
Update to the latest stable Debian. While we are at it flatten into a single dockerfile. We really don't need the rest of the stuff from the QEMU base image just to compile test images. Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20220826172128.353798-8-alex.ben...@linaro.org> --- .gitlab-ci.d/container-cross.yml | 1 - tests/docker/Makefile.include| 1 - tests/docker/dockerfiles/debian-sh4-cross.docker | 12 +++- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml index a3bfa483bf..8a611fc824 100644 --- a/.gitlab-ci.d/container-cross.yml +++ b/.gitlab-ci.d/container-cross.yml @@ -137,7 +137,6 @@ s390x-debian-cross-container: sh4-debian-cross-container: extends: .container_job_template stage: containers - needs: ['amd64-debian10-container'] variables: NAME: debian-sh4-cross diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 6c2ee3b175..37c4ea913f 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -89,7 +89,6 @@ DOCKER_PARTIAL_IMAGES += fedora endif docker-image-debian-mips-cross: docker-image-debian10 -docker-image-debian-sh4-cross: docker-image-debian10 docker-image-debian-sparc64-cross: docker-image-debian10 # The native build should never use the registry diff --git a/tests/docker/dockerfiles/debian-sh4-cross.docker b/tests/docker/dockerfiles/debian-sh4-cross.docker index fd3af89575..d48ed9065f 100644 --- a/tests/docker/dockerfiles/debian-sh4-cross.docker +++ b/tests/docker/dockerfiles/debian-sh4-cross.docker @@ -1,12 +1,14 @@ # # Docker cross-compiler target # -# This docker target builds on the debian Buster base image. +# This docker target builds on the Debian Bullseye base image. # -FROM qemu/debian10 +FROM docker.io/library/debian:11-slim -RUN apt update && \ -DEBIAN_FRONTEND=noninteractive eatmydata \ -apt install -y --no-install-recommends \ +RUN export DEBIAN_FRONTEND=noninteractive && \ +apt-get update && \ +apt-get install -y eatmydata && \ +eatmydata apt-get dist-upgrade -y && \ +eatmydata apt-get install --no-install-recommends -y \ gcc-sh4-linux-gnu \ libc6-dev-sh4-cross -- 2.34.1
[PATCH v2 08/30] tests/docker: update and flatten debian-alpha-cross
Update to the latest stable Debian. While we are at it flatten into a single dockerfile. We really don't need the rest of the stuff from the QEMU base image just to compile test images. Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20220826172128.353798-4-alex.ben...@linaro.org> --- .gitlab-ci.d/container-cross.yml | 1 - tests/docker/Makefile.include | 1 - tests/docker/dockerfiles/debian-alpha-cross.docker | 12 +++- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml index 505b267542..802e332205 100644 --- a/.gitlab-ci.d/container-cross.yml +++ b/.gitlab-ci.d/container-cross.yml @@ -1,7 +1,6 @@ alpha-debian-cross-container: extends: .container_job_template stage: containers - needs: ['amd64-debian10-container'] variables: NAME: debian-alpha-cross diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 9a45e8890b..c565aa5e7b 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -88,7 +88,6 @@ DOCKER_PARTIAL_IMAGES += debian-s390x-cross DOCKER_PARTIAL_IMAGES += fedora endif -docker-image-debian-alpha-cross: docker-image-debian10 docker-image-debian-hppa-cross: docker-image-debian10 docker-image-debian-m68k-cross: docker-image-debian10 docker-image-debian-mips-cross: docker-image-debian10 diff --git a/tests/docker/dockerfiles/debian-alpha-cross.docker b/tests/docker/dockerfiles/debian-alpha-cross.docker index 10fe30df0d..4eeb43c78a 100644 --- a/tests/docker/dockerfiles/debian-alpha-cross.docker +++ b/tests/docker/dockerfiles/debian-alpha-cross.docker @@ -1,12 +1,14 @@ # # Docker cross-compiler target # -# This docker target builds on the debian Buster base image. +# This docker target builds on the Debian Bullseye base image. # -FROM qemu/debian10 +FROM docker.io/library/debian:11-slim -RUN apt update && \ -DEBIAN_FRONTEND=noninteractive eatmydata \ -apt install -y --no-install-recommends \ +RUN export DEBIAN_FRONTEND=noninteractive && \ +apt-get update && \ +apt-get install -y eatmydata && \ +eatmydata apt-get dist-upgrade -y && \ +eatmydata apt-get install --no-install-recommends -y \ gcc-alpha-linux-gnu \ libc6.1-dev-alpha-cross -- 2.34.1
[PATCH v2 16/30] tests/docker: remove amd64 qemu/debian10 dependency
We missed removing this dependency when we flattened the build. Fixes 9e19fd7d4a (tests/docker: update debian-amd64 with lcitool) Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20220826172128.353798-12-alex.ben...@linaro.org> --- tests/docker/Makefile.include | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 5c9398bbc9..c3375f89c5 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -73,7 +73,6 @@ docker-binfmt-image-debian-%: $(DOCKER_FILES_DIR)/debian-bootstrap.docker # we don't run tests on intermediate images (used as base by another image) DOCKER_PARTIAL_IMAGES := debian10 debian11 ifeq ($(HOST_ARCH),x86_64) -docker-image-debian-amd64: docker-image-debian10 DOCKER_PARTIAL_IMAGES += debian-amd64-cross else docker-image-debian-amd64-cross: docker-image-debian10 -- 2.34.1
[PATCH v2 07/30] tests/avocado: reduce the default timeout to 120s
We should be aiming to keep our tests under 2 minutes so lets reduce the default timeout to that. Tests that we know take longer should explicitly set a longer timeout. Signed-off-by: Alex Bennée Message-Id: <20220826172128.353798-3-alex.ben...@linaro.org> --- tests/avocado/avocado_qemu/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py index 5f1d3e551c..910f3ba1ea 100644 --- a/tests/avocado/avocado_qemu/__init__.py +++ b/tests/avocado/avocado_qemu/__init__.py @@ -229,7 +229,7 @@ def exec_command_and_wait_for_pattern(test, command, class QemuBaseTest(avocado.Test): # default timeout for all tests, can be overridden -timeout = 900 +timeout = 120 def _get_unique_tag_val(self, tag_name): """ -- 2.34.1
[PATCH v2 10/30] tests/docker: update and flatten debian-m68k-cross
Update to the latest stable Debian. While we are at it flatten into a single dockerfile. We really don't need the rest of the stuff from the QEMU base image just to compile test images. Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20220826172128.353798-6-alex.ben...@linaro.org> --- .gitlab-ci.d/container-cross.yml | 1 - tests/docker/Makefile.include | 1 - tests/docker/dockerfiles/debian-m68k-cross.docker | 12 +++- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml index 6c1d765463..15a5270f6d 100644 --- a/.gitlab-ci.d/container-cross.yml +++ b/.gitlab-ci.d/container-cross.yml @@ -71,7 +71,6 @@ hppa-debian-cross-container: m68k-debian-cross-container: extends: .container_job_template stage: containers - needs: ['amd64-debian10-container'] variables: NAME: debian-m68k-cross diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index e39597d35c..95790e974e 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -88,7 +88,6 @@ DOCKER_PARTIAL_IMAGES += debian-s390x-cross DOCKER_PARTIAL_IMAGES += fedora endif -docker-image-debian-m68k-cross: docker-image-debian10 docker-image-debian-mips-cross: docker-image-debian10 docker-image-debian-mips64-cross: docker-image-debian10 docker-image-debian-sh4-cross: docker-image-debian10 diff --git a/tests/docker/dockerfiles/debian-m68k-cross.docker b/tests/docker/dockerfiles/debian-m68k-cross.docker index fcb10e3534..dded71c5d2 100644 --- a/tests/docker/dockerfiles/debian-m68k-cross.docker +++ b/tests/docker/dockerfiles/debian-m68k-cross.docker @@ -1,12 +1,14 @@ # # Docker cross-compiler target # -# This docker target builds on the debian Buster base image. +# This docker target builds on the Debian Bullseye base image. # -FROM qemu/debian10 +FROM docker.io/library/debian:11-slim -RUN apt update && \ -DEBIAN_FRONTEND=noninteractive eatmydata \ -apt install -y --no-install-recommends \ +RUN export DEBIAN_FRONTEND=noninteractive && \ +apt-get update && \ +apt-get install -y eatmydata && \ +eatmydata apt-get dist-upgrade -y && \ +eatmydata apt-get install --no-install-recommends -y \ gcc-m68k-linux-gnu \ libc6-dev-m68k-cross -- 2.34.1
[PATCH v2 03/30] tests/avocado: add explicit timeout for Aarch64 TCG tests
We don't want to rely on the soon to be reduced default time. These
tests are still slow for something we want to run in CI though.
Signed-off-by: Alex Bennée
---
tests/avocado/boot_linux.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
index ee584d2fdf..67a24fe51c 100644
--- a/tests/avocado/boot_linux.py
+++ b/tests/avocado/boot_linux.py
@@ -63,6 +63,7 @@ class BootLinuxAarch64(LinuxTest):
:avocado: tags=machine:virt
:avocado: tags=machine:gic-version=2
"""
+timeout = 240
def add_common_args(self):
self.vm.add_args('-bios',
--
2.34.1
[PATCH v2 09/30] tests/docker: update and flatten debian-hppa-cross
Update to the latest stable Debian. While we are at it flatten into a single dockerfile. We really don't need the rest of the stuff from the QEMU base image just to compile test images. Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20220826172128.353798-5-alex.ben...@linaro.org> --- .gitlab-ci.d/container-cross.yml | 1 - tests/docker/Makefile.include | 1 - tests/docker/dockerfiles/debian-hppa-cross.docker | 12 +++- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml index 802e332205..6c1d765463 100644 --- a/.gitlab-ci.d/container-cross.yml +++ b/.gitlab-ci.d/container-cross.yml @@ -65,7 +65,6 @@ hexagon-cross-container: hppa-debian-cross-container: extends: .container_job_template stage: containers - needs: ['amd64-debian10-container'] variables: NAME: debian-hppa-cross diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index c565aa5e7b..e39597d35c 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -88,7 +88,6 @@ DOCKER_PARTIAL_IMAGES += debian-s390x-cross DOCKER_PARTIAL_IMAGES += fedora endif -docker-image-debian-hppa-cross: docker-image-debian10 docker-image-debian-m68k-cross: docker-image-debian10 docker-image-debian-mips-cross: docker-image-debian10 docker-image-debian-mips64-cross: docker-image-debian10 diff --git a/tests/docker/dockerfiles/debian-hppa-cross.docker b/tests/docker/dockerfiles/debian-hppa-cross.docker index 3d6c65a3ef..af1c8403d8 100644 --- a/tests/docker/dockerfiles/debian-hppa-cross.docker +++ b/tests/docker/dockerfiles/debian-hppa-cross.docker @@ -1,12 +1,14 @@ # # Docker cross-compiler target # -# This docker target builds on the debian Buster base image. +# This docker target builds on the Debian Bullseye base image. # -FROM qemu/debian10 +FROM docker.io/library/debian:11-slim -RUN apt update && \ -DEBIAN_FRONTEND=noninteractive eatmydata \ -apt install -y --no-install-recommends \ +RUN export DEBIAN_FRONTEND=noninteractive && \ +apt-get update && \ +apt-get install -y eatmydata && \ +eatmydata apt-get dist-upgrade -y && \ +eatmydata apt-get install --no-install-recommends -y \ gcc-hppa-linux-gnu \ libc6-dev-hppa-cross -- 2.34.1
[PATCH v2 04/30] tests/avocado: add explicit timeout for s390 TCG tests
We don't want to rely on the soon to be reduced default time. These
tests are still slow for something we want to run in CI though.
Signed-off-by: Alex Bennée
---
tests/avocado/boot_linux.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py
index 67a24fe51c..4f07c27ac6 100644
--- a/tests/avocado/boot_linux.py
+++ b/tests/avocado/boot_linux.py
@@ -130,6 +130,8 @@ class BootLinuxS390X(LinuxTest):
:avocado: tags=arch:s390x
"""
+timeout = 240
+
@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
def test_s390_ccw_virtio_tcg(self):
"""
--
2.34.1
[PATCH v2 06/30] tests/avocado: split the AST2x00Machine classes
The SDK tests take a lot longer to run and hence need a longer
timeout. As they run well over the 60 second maximum for CI lets also
disable them for CI as well.
I suspect they also suffer from the inability to detect the login
prompt due to no newlines being processed.
Signed-off-by: Alex Bennée
Reviewed-by: Thomas Huth
Message-Id: <20220826172128.353798-2-alex.ben...@linaro.org>
---
tests/avocado/machine_aspeed.py | 18 ++
1 file changed, 18 insertions(+)
diff --git a/tests/avocado/machine_aspeed.py b/tests/avocado/machine_aspeed.py
index 65d38f4efa..0f64eb636c 100644
--- a/tests/avocado/machine_aspeed.py
+++ b/tests/avocado/machine_aspeed.py
@@ -6,12 +6,14 @@
# later. See the COPYING file in the top-level directory.
import time
+import os
from avocado_qemu import QemuSystemTest
from avocado_qemu import wait_for_console_pattern
from avocado_qemu import exec_command
from avocado_qemu import exec_command_and_wait_for_pattern
from avocado.utils import archive
+from avocado import skipIf
class AST1030Machine(QemuSystemTest):
@@ -176,6 +178,20 @@ def test_arm_ast2600_evb_builroot(self):
self.do_test_arm_aspeed_buidroot_poweroff()
+class AST2x00MachineSDK(QemuSystemTest):
+
+# FIXME: Although these tests boot a whole distro they are still
+# slower than comparable machine models. There may be some
+# optimisations which bring down the runtime. In the meantime they
+# have generous timeouts and are disable for CI which aims for all
+# tests to run in less than 60 seconds.
+timeout = 240
+
+def wait_for_console_pattern(self, success_message, vm=None):
+wait_for_console_pattern(self, success_message,
+ failure_message='Kernel panic - not syncing',
+ vm=vm)
+
def do_test_arm_aspeed_sdk_start(self, image, cpu_id):
self.vm.set_console()
self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
@@ -187,6 +203,7 @@ def do_test_arm_aspeed_sdk_start(self, image, cpu_id):
self.wait_for_console_pattern('Starting kernel ...')
self.wait_for_console_pattern('Booting Linux on physical CPU ' +
cpu_id)
+@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
def test_arm_ast2500_evb_sdk(self):
"""
:avocado: tags=arch:arm
@@ -204,6 +221,7 @@ def test_arm_ast2500_evb_sdk(self):
self.workdir + '/ast2500-default/image-bmc', '0x0')
self.wait_for_console_pattern('ast2500-default login:')
+@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
def test_arm_ast2600_evb_sdk(self):
"""
:avocado: tags=arch:arm
--
2.34.1
[PATCH v2 00/30] testing/next pre-PR (testing update and mips deprecation)
Hi, This is the current state of the testing tree. It was slightly delayed from the last posting as I was waiting for the libvirt-ci update to get merged. Changes since last post: - added some more avocado timeouts - moved cflags hack in the --disable-pie patch into configure - usual addressing of review comments (see --- in patches) I'd like to roll the PR next week so please review the remaining un-reviewed patches: - tests/docker: remove FROM qemu/ support from docker.py - tests/docker: update and flatten debian-hexagon-cross - tests/docker: update and flatten debian-amd64-cross - tests/docker: flatten debian-riscv64-test-cross - configure: explicitly set cflags for --disable-pie - tests/docker: remove tricore qemu/debian10 dependency - tests/avocado: reduce the default timeout to 120s - tests/avocado: add explicit timeout for ppc64le TCG tests - tests/avocado: add explicit timeout for s390 TCG tests - tests/avocado: add explicit timeout for Aarch64 TCG tests - gitlab: reduce targets in cross_user_build_job Alex Bennée (27): gitlab: reduce targets in cross_user_build_job tests/avocado: add explicit timeout for Aarch64 TCG tests tests/avocado: add explicit timeout for s390 TCG tests tests/avocado: add explicit timeout for ppc64le TCG tests tests/avocado: split the AST2x00Machine classes tests/avocado: reduce the default timeout to 120s tests/docker: update and flatten debian-alpha-cross tests/docker: update and flatten debian-hppa-cross tests/docker: update and flatten debian-m68k-cross tests/docker: update and flatten debian-mips64-cross tests/docker: update and flatten debian-sh4-cross tests/docker: update and flatten debian-sparc64-cross tests/docker: flatten debian-powerpc-test-cross tests/docker: remove tricore qemu/debian10 dependency tests/docker: remove amd64 qemu/debian10 dependency configure: explicitly set cflags for --disable-pie gitlab-ci: update aarch32/aarch64 custom runner jobs Deprecate 32 bit big-endian MIPS tests/docker: flatten debian-riscv64-test-cross tests/docker: update and flatten debian-all-test-cross tests/lcitool: bump to latest version tests/docker: update and flatten debian-amd64-cross tests/docker: update and flatten debian-loongarch-cross tests/docker: update and flatten debian-hexagon-cross tests/docker: update and flatten debian-toolchain tests/docker: remove FROM qemu/ support from docker.py tests/docker: remove the Debian base images Richard Henderson (1): gitlab-ci/custom-runners: Disable -static-pie for ubuntu-20.04-aarch64 Thomas Huth (2): tests/avocado/boot_linux_console: Fix the test_aarch64_xlnx_versal_virt test tests/vm: Remove obsolete Fedora VM test docs/about/build-platforms.rst| 2 +- docs/about/deprecated.rst | 13 ++ docs/devel/testing.rst| 2 +- configure | 6 + .gitlab-ci.d/cirrus/freebsd-12.vars | 2 +- .gitlab-ci.d/cirrus/freebsd-13.vars | 2 +- .gitlab-ci.d/container-core.yml | 5 - .gitlab-ci.d/container-cross.yml | 12 -- .gitlab-ci.d/containers.yml | 5 - .gitlab-ci.d/crossbuild-template.yml | 5 +- .gitlab-ci.d/crossbuilds.yml | 14 -- .gitlab-ci.d/custom-runners.yml | 4 +- ...4-aarch32.yml => ubuntu-22.04-aarch32.yml} | 6 +- ...4-aarch64.yml => ubuntu-22.04-aarch64.yml} | 38 ++-- MAINTAINERS | 3 +- tests/avocado/avocado_qemu/__init__.py| 2 +- tests/avocado/boot_linux.py | 5 + tests/avocado/boot_linux_console.py | 6 +- tests/avocado/machine_aspeed.py | 18 ++ tests/docker/Makefile.include | 30 +-- tests/docker/docker.py| 38 +--- .../dockerfiles/debian-all-test-cross.docker | 18 +- .../dockerfiles/debian-alpha-cross.docker | 12 +- .../dockerfiles/debian-amd64-cross.docker | 178 ++-- .../dockerfiles/debian-hexagon-cross.docker | 19 +- .../dockerfiles/debian-hppa-cross.docker | 12 +- .../dockerfiles/debian-loongarch-cross.docker | 8 +- .../dockerfiles/debian-m68k-cross.docker | 12 +- .../dockerfiles/debian-mips-cross.docker | 38 +--- .../dockerfiles/debian-mips64-cross.docker| 12 +- .../debian-powerpc-test-cross.docker | 12 +- .../debian-riscv64-test-cross.docker | 10 +- .../dockerfiles/debian-sh4-cross.docker | 12 +- .../dockerfiles/debian-sparc64-cross.docker | 12 +- .../dockerfiles/debian-toolchain.docker | 5 +- tests/docker/dockerfiles/debian10.docker | 38 tests/docker/dockerfiles/debian11.docker | 18 -- tests/docker/dockerfiles/opensuse-leap.docker | 3 +- tests/docker/dockerfiles/ubuntu2004.docker| 2 +- tests/lcitool/libvirt-ci | 2 +-
[PATCH v2 02/30] tests/avocado/boot_linux_console: Fix the test_aarch64_xlnx_versal_virt test
From: Thomas Huth
The assets that this test tries to download have been removed from the
server. Update to a newer version to get it working again.
Signed-off-by: Thomas Huth
Reviewed-by: Alistair Francis
Message-Id: <20220829080940.110831-1-th...@redhat.com>
Signed-off-by: Alex Bennée
---
tests/avocado/boot_linux_console.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/avocado/boot_linux_console.py
b/tests/avocado/boot_linux_console.py
index 6b1533c17c..f26e036ab5 100644
--- a/tests/avocado/boot_linux_console.py
+++ b/tests/avocado/boot_linux_console.py
@@ -335,13 +335,13 @@ def test_aarch64_xlnx_versal_virt(self):
"""
images_url = ('http://ports.ubuntu.com/ubuntu-ports/dists/'
'bionic-updates/main/installer-arm64/'
- '20101020ubuntu543.15/images/')
+ '20101020ubuntu543.19/images/')
kernel_url = images_url + 'netboot/ubuntu-installer/arm64/linux'
-kernel_hash = '5bfc54cf7ed8157d93f6e5b0241e727b6dc22c50'
+kernel_hash = 'e167757620640eb26de0972f578741924abb3a82'
kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
initrd_url = images_url + 'netboot/ubuntu-installer/arm64/initrd.gz'
-initrd_hash = 'd385d3e88d53e2004c5d43cbe668b458a094f772'
+initrd_hash = 'cab5cb3fcefca8408aa5aae57f24574bfce8bdb9'
initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
self.vm.set_console()
--
2.34.1
[PATCH v2 01/30] gitlab: reduce targets in cross_user_build_job
We already limit the scope of the cross system build to reduce the cross build times. With the recent addition of more targets we are also running into timeout issues for some of the cross user builds. I've selected a few of those linux-user targets which are less likely to be in common use as distros don't have pre-built rootfs for them. I've also added the same CROSS_SKIP_TARGETS variable as is occasionally used to further limit cross system builds. Signed-off-by: Alex Bennée --- .gitlab-ci.d/crossbuild-template.yml | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.d/crossbuild-template.yml b/.gitlab-ci.d/crossbuild-template.yml index 28b2142ec2..5e8892fd49 100644 --- a/.gitlab-ci.d/crossbuild-template.yml +++ b/.gitlab-ci.d/crossbuild-template.yml @@ -46,5 +46,8 @@ - cd build - PKG_CONFIG_PATH=$PKG_CONFIG_PATH ../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS ---disable-system +--disable-system --target-list-exclude="aarch64_be-linux-user + alpha-linux-user cris-linux-user m68k-linux-user microblazeel-linux-user + nios2-linux-user or1k-linux-user ppc-linux-user sparc-linux-user + xtensa-linux-user $CROSS_SKIP_TARGETS" - make -j$(expr $(nproc) + 1) all check-build $MAKE_CHECK_ARGS -- 2.34.1
Re: [PATCH 2/3] vdpa: load vlan configuration at NIC startup
On 9/14/2022 2:57 PM, Eugenio Perez Martin wrote:
On Wed, Sep 14, 2022 at 1:33 PM Si-Wei Liu wrote:
On 9/14/2022 3:20 AM, Jason Wang wrote:
On Fri, Sep 9, 2022 at 4:02 PM Eugenio Perez Martin wrote:
On Fri, Sep 9, 2022 at 8:40 AM Jason Wang wrote:
On Fri, Sep 9, 2022 at 2:38 PM Jason Wang wrote:
On Wed, Sep 7, 2022 at 12:36 AM Eugenio Pérez wrote:
To have enabled vlans at device startup may happen in the destination of
a live migration, so this configuration must be restored.
At this moment the code is not accessible, since SVQ refuses to start if
vlan feature is exposed by the device.
Signed-off-by: Eugenio Pérez
---
net/vhost-vdpa.c | 46 --
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 4bc3fd01a8..ecbfd08eb9 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -100,6 +100,8 @@ static const uint64_t vdpa_svq_device_features =
BIT_ULL(VIRTIO_NET_F_RSC_EXT) |
BIT_ULL(VIRTIO_NET_F_STANDBY);
+#define MAX_VLAN(1 << 12) /* Per 802.1Q definition */
+
VHostNetState *vhost_vdpa_get_vhost_net(NetClientState *nc)
{
VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
@@ -423,6 +425,47 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState *s,
return *s->status != VIRTIO_NET_OK;
}
+static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
+ const VirtIONet *n,
+ uint16_t vid)
+{
+ssize_t dev_written = vhost_vdpa_net_load_cmd(s, VIRTIO_NET_CTRL_VLAN,
+ VIRTIO_NET_CTRL_VLAN_ADD,
+ , sizeof(vid));
+if (unlikely(dev_written < 0)) {
+return dev_written;
+}
+
+if (unlikely(*s->status != VIRTIO_NET_OK)) {
+return -EINVAL;
+}
+
+return 0;
+}
+
+static int vhost_vdpa_net_load_vlan(VhostVDPAState *s,
+const VirtIONet *n)
+{
+uint64_t features = n->parent_obj.guest_features;
+
+if (!(features & BIT_ULL(VIRTIO_NET_F_CTRL_VLAN))) {
+return 0;
+}
+
+for (int i = 0; i < MAX_VLAN >> 5; i++) {
+for (int j = 0; n->vlans[i] && j <= 0x1f; j++) {
+if (n->vlans[i] & (1U << j)) {
+int r = vhost_vdpa_net_load_single_vlan(s, n, (i << 5) + j);
This seems to cause a lot of latency if the driver has a lot of vlans.
I wonder if it's simply to let all vlan traffic go by disabling
CTRL_VLAN feature at vDPA layer.
The guest will not be able to recover that vlan configuration.
Spec said it's best effort and we actually don't do this for
vhost-net/user for years and manage to survive.
I thought there's a border line between best effort and do nothing. ;-)
I also think it is worth at least trying to migrate it for sure. For
MAC there may be too many entries, but VLAN should be totally
manageable (and the information is already there, the bitmap is
actually being migrated).
The vlan bitmap is migrated, though you'd still need to add vlan filter
one by one through ctrl vq commands, correct? AFAIK you can add one
filter for one single vlan ID at a time via VIRTIO_NET_CTRL_VLAN_ADD.
I think that the reason this could survive is really software
implementation specific - say if the backend doesn't start with VLAN
filtering disabled (meaning allow all VLAN traffic to pass) then it
would become a problem. This won't be a problem for almost PF NICs but
may be problematic for vDPA e.g. VF backed VDPAs.
I'd say the driver would expect all vlan filters to be cleared after a
reset, isn't it?
If I understand the intended behavior (from QEMU implementation)
correctly, when VIRTIO_NET_F_CTRL_VLAN is negotiated it would start with
all VLANs filtered (meaning only untagged traffic can be received and
traffic with VLAN tag would get dropped). However, if
VIRTIO_NET_F_CTRL_VLAN is not negotiated, all traffic including untagged
and tagged can be received.
The spec doesn't explicitly say anything about that
as far as I see.
Here the spec is totally ruled by the (software artifact of)
implementation rather than what a real device is expected to work with
VLAN rx filters. Are we sure we'd stick to this flawed device
implementation? The guest driver seems to be agnostic with this broken
spec behavior so far, and I am afraid it's an overkill to add another
feature bit or ctrl command to VLAN filter in clean way.
Another idea is to extend the spec to allow us to accept a bitmap of
the vlan ids via a single command, then we will be fine.
I'm not sure if adding more ways to configure something is the answer,
but I'd be ok to implement it.
Another idea is to allow the sending of many CVQ commands in parallel.
It shouldn't be very hard to achieve using exposed buffers as ring
buffers, and it will short down the start of the devices with many
features.
Re: [PATCH v11 20/21] blockjob: remove unused functions
On 8/26/22 16:21, Emanuele Giuseppe Esposito wrote: These public functions are not used anywhere, thus can be dropped. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi Reviewed-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
Re: [PATCH v11 21/21] job: remove unused functions
On 8/26/22 16:21, Emanuele Giuseppe Esposito wrote: These public functions are not used anywhere, thus can be dropped. Also, since this is the final job API that doesn't use AioContext lock and replaces it with job_lock, adjust all remaining function documentation to clearly specify if the job lock is taken or not. Also document the locking requirements for a few functions where the second version is not removed. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi Reviewed-by: Kevin Wolf --- include/qemu/job.h | 96 ++ job.c | 101 ++--- 2 files changed, 34 insertions(+), 163 deletions(-) diff --git a/include/qemu/job.h b/include/qemu/job.h index ad8b32b4ba..762d6a98a7 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -384,6 +384,8 @@ JobTxn *job_txn_new(void); /** * Release a reference that was previously acquired with job_txn_add_job or [..] +/** + * Returns true if the job is user-paused. + * Called with job lock held. + */ bool job_user_paused_locked(Job *job); /** * Resume the specified @job. * Must be paired with a preceding job_user_pause. in comment: job_user_puase_locked Please also fix other removed function mentioning in comments, for example I see in comments mentioning of removed job_ref, job_unref, job_user_pause... - */ -void job_user_resume(Job *job, Error **errp); - -/* - * Same as job_user_resume(), but called with job lock held. - * Might release the lock temporarily. + * Called with job lock held, but might release it temporarily. */ void job_user_resume_locked(Job *job, Error **errp); @@ -624,6 +611,7 @@ void job_user_resume_locked(Job *job, Error **errp); * first one if @job is %NULL. [..] /** @@ -725,9 +703,6 @@ void job_cancel_sync_all(void); * Returns the return value from the job. * Called with job_lock *not* held. in comment: with lock held. */ -int job_complete_sync(Job *job, Error **errp); - -/* Same as job_complete_sync, but called with job lock held. */ int job_complete_sync_locked(Job *job, Error **errp); -- Best regards, Vladimir
Re: [PATCH v3 03/20] ppc4xx_sdram: Get rid of the init RAM hack
On 9/14/22 13:44, BALATON Zoltan wrote:
On Wed, 14 Sep 2022, Cédric Le Goater wrote:
On 9/13/22 21:52, BALATON Zoltan wrote:
The do_init parameter of ppc4xx_sdram_init() is used to map memory
regions that is normally done by the firmware by programming the SDRAM
controller. This is needed when booting a kernel directly from -kernel
without a firmware. Do this from board code accesing normal SDRAM
accessing
Fixed, also two ofhers in another patch you haven't noticed.
controller registers the same way as firmware would do, so we can get
rid of this hack.
Signed-off-by: BALATON Zoltan
---
v2: Fix ref405ep boot with -kernel and U-Boot
hw/ppc/ppc405.h | 1 -
hw/ppc/ppc405_boards.c | 12 ++--
hw/ppc/ppc405_uc.c | 4 +---
hw/ppc/ppc440_bamboo.c | 8 +++-
hw/ppc/ppc440_uc.c | 2 --
hw/ppc/ppc4xx_devs.c | 11 +--
include/hw/ppc/ppc4xx.h | 8 ++--
7 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index 1e558c7831..756865621b 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -169,7 +169,6 @@ struct Ppc405SoCState {
/* Public */
MemoryRegion ram_banks[2];
hwaddr ram_bases[2], ram_sizes[2];
- bool do_dram_init;
MemoryRegion *dram_mr;
hwaddr ram_size;
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 083f12b23e..bf02a71c6d 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -274,6 +274,7 @@ static void ppc405_init(MachineState *machine)
MachineClass *mc = MACHINE_GET_CLASS(machine);
const char *kernel_filename = machine->kernel_filename;
MemoryRegion *sysmem = get_system_memory();
+ CPUPPCState *env;
if (machine->ram_size != mc->default_ram_size) {
char *sz = size_to_str(mc->default_ram_size);
@@ -288,12 +289,19 @@ static void ppc405_init(MachineState *machine)
machine->ram_size, _fatal);
object_property_set_link(OBJECT(>soc), "dram",
OBJECT(machine->ram), _abort);
- object_property_set_bool(OBJECT(>soc), "dram-init",
- kernel_filename != NULL, _abort);
object_property_set_uint(OBJECT(>soc), "sys-clk", ,
_abort);
qdev_realize(DEVICE(>soc), NULL, _fatal);
+ /* Enable SDRAM memory regions */
+ /* FIXME This shouldn't be needed with firmware but we lack SPD data */
what do you mean ?
U-Boot detects the available RAM by reading the SPD info of the RAM modules but
that probably also needs i2c emulation. See sam460ex.
+ env = >soc.cpu.env;
+ if (ppc_dcr_write(env->dcr_env, SDRAM0_CFGADDR, 0x20) ||
+ ppc_dcr_write(env->dcr_env, SDRAM0_CFGDATA, 0x8000)) {
I am not in favor of these ppc_drc_write calls and this is still a hack.
It's not. Normally this is done by firmware to enable memory controller but the
board code has to do it if not using firmware (e.g. booting with -kernel) the
same way it provides bootinfo or device tree mods the firmware would normally
do or in this case maybe the emulation is incomplete so the part of firmware
that configures the SDRAM controller does not run.
Exactly, and what the above proposal does is mimicking execution of CPU
instructions before the CPU is even fully initiated. Reset has not been
called at that stage.
The "dram-init" property is a cleaner solution. It takes care of doing the
pre-mapping of RAM banks in the realize routine of the sdram model (when
available).
I disagree, the hardware does not have such feature, it proviesd DCRs as the way to configure it. Adding a special property for it deviates from hardware and clutters qtree.
In this machine, running QEMU with -kernel deviates from HW. That's
the whole purpose of this option. It assumes that the SDRAM device
is pre-initialized (and much more should be done) by the QEMU machine
and the simplest way to acheive this goal is to inform the SDRAM model
to take care of the pre-initialization.
Another way would be to change the default reset values of the SDRAM
registers (in the realize method using some property) and perform
some actions (mapping the banks) in the reset handler of the SDRAM
device model. That would be a deferred initialization but a property
is still needed to change the default behavior of the SDRAM model.
Anyhow, this should be isolated under the SDRAM device model and
not in the machine init by using the CPU state. That's clearly ugly.
Thanks,
C.
Doing it like this patch is cleaner IMO.
Regards,
BALATON Zoltan
C.
+ error_report("Could not enable memory regions");
+ exit(1);
+ }
+
/* allocate and load BIOS */
if (machine->firmware) {
MemoryRegion *bios = g_new(MemoryRegion, 1);
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index 2ca42fdef6..1e02347e57 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1081,8 +1081,7
[PULL 11/11] target/riscv: Honour -semihosting-config userspace=on and enable=on
From: Peter Maydell
The riscv target incorrectly enabled semihosting always, whether the
user asked for it or not. Call semihosting_enabled() passing the
correct value to the is_userspace argument, which fixes this and also
handles the userspace=on argument. Because we do this at translate
time, we no longer need to check the privilege level in
riscv_cpu_do_interrupt().
Note that this is a behaviour change: we used to default to
semihosting being enabled, and now the user must pass
"-semihosting-config enable=on" if they want it.
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Reviewed-by: Alistair Francis
Message-Id: <20220822141230.3658237-8-peter.mayd...@linaro.org>
Signed-off-by: Richard Henderson
---
target/riscv/cpu_helper.c | 9 +++--
target/riscv/translate.c | 1 +
target/riscv/insn_trans/trans_privileged.c.inc | 3 ++-
3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 67e4c0efd2..278d163803 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1589,12 +1589,9 @@ void riscv_cpu_do_interrupt(CPUState *cs)
target_ulong mtval2 = 0;
if (cause == RISCV_EXCP_SEMIHOST) {
-if (env->priv >= PRV_S) {
-do_common_semihosting(cs);
-env->pc += 4;
-return;
-}
-cause = RISCV_EXCP_BREAKPOINT;
+do_common_semihosting(cs);
+env->pc += 4;
+return;
}
if (!async) {
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 8925a44c6e..db123da5ec 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -28,6 +28,7 @@
#include "exec/translator.h"
#include "exec/log.h"
+#include "semihosting/semihost.h"
#include "instmap.h"
#include "internals.h"
diff --git a/target/riscv/insn_trans/trans_privileged.c.inc
b/target/riscv/insn_trans/trans_privileged.c.inc
index 46f96ad74d..3281408a87 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -52,7 +52,8 @@ static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
* that no exception will be raised when fetching them.
*/
-if ((pre_addr & TARGET_PAGE_MASK) == (post_addr & TARGET_PAGE_MASK)) {
+if (semihosting_enabled(ctx->mem_idx < PRV_S) &&
+(pre_addr & TARGET_PAGE_MASK) == (post_addr & TARGET_PAGE_MASK)) {
pre= opcode_at(>base, pre_addr);
ebreak = opcode_at(>base, ebreak_addr);
post = opcode_at(>base, post_addr);
--
2.34.1
Re: [PATCH v11 13/21] jobs: protect job.aio_context with BQL and job_mutex
On 8/26/22 16:20, Emanuele Giuseppe Esposito wrote:
In order to make it thread safe, implement a "fake rwlock",
where we allow reads under BQL *or* job_mutex held, but
writes only under BQL *and* job_mutex.
The only write we have is in child_job_set_aio_ctx, which always
happens under drain (so the job is paused).
For this reason, introduce job_set_aio_context and make sure that
the context is set under BQL, job_mutex and drain.
Also make sure all other places where the aiocontext is read
are protected.
The reads in commit.c and mirror.c are actually safe, because always
done under BQL.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Suggested-by: Paolo Bonzini
Signed-off-by: Emanuele Giuseppe Esposito
---
block/replication.c | 7 +--
blockjob.c | 3 ++-
include/qemu/job.h | 23 ---
job.c | 12
4 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/block/replication.c b/block/replication.c
index 55c8f894aa..6e02d98126 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -142,14 +142,17 @@ static void replication_close(BlockDriverState *bs)
{
BDRVReplicationState *s = bs->opaque;
Job *commit_job;
+GLOBAL_STATE_CODE();
if (s->stage == BLOCK_REPLICATION_RUNNING) {
replication_stop(s->rs, false, NULL);
}
if (s->stage == BLOCK_REPLICATION_FAILOVER) {
commit_job = >commit_job->job;
-assert(commit_job->aio_context == qemu_get_current_aio_context());
-job_cancel_sync(commit_job, false);
+WITH_JOB_LOCK_GUARD() {
+assert(commit_job->aio_context == qemu_get_current_aio_context());
+job_cancel_sync_locked(commit_job, false);
+}
As Kevin said, this hunk seems not needed.. Why to add locking for reading
aio_context, when we have GLOBAL_STATE_CODE()?
}
if (s->mode == REPLICATION_MODE_SECONDARY) {
diff --git a/blockjob.c b/blockjob.c
index 96fb9d9f73..c8919cef9b 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -162,12 +162,13 @@ static void child_job_set_aio_ctx(BdrvChild *c,
AioContext *ctx,
bdrv_set_aio_context_ignore(sibling->bs, ctx, ignore);
}
-job->job.aio_context = ctx;
+job_set_aio_context(>job, ctx);
}
static AioContext *child_job_get_parent_aio_context(BdrvChild *c)
{
BlockJob *job = c->opaque;
+GLOBAL_STATE_CODE();
return job->job.aio_context;
}
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 5709e8d4a8..cede227e67 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -74,11 +74,17 @@ typedef struct Job {
/* ProgressMeter API is thread-safe */
ProgressMeter progress;
+/**
+ * AioContext to run the job coroutine in.
+ * The job Aiocontext can be read when holding *either*
+ * the BQL (so we are in the main loop) or the job_mutex.
+ * It can only be written when we hold *both* BQL
+ * and the job_mutex.
+ */
+AioContext *aio_context;
-/** Protected by AioContext lock */
-/** AioContext to run the job coroutine in */
-AioContext *aio_context;
+/** Protected by AioContext lock */
/** Reference count of the block job */
int refcnt;
@@ -741,4 +747,15 @@ int job_finish_sync(Job *job, void (*finish)(Job *, Error
**errp),
int job_finish_sync_locked(Job *job, void (*finish)(Job *, Error **errp),
Error **errp);
+/**
+ * Sets the @job->aio_context.
+ * Called with job_mutex *not* held.
+ *
+ * This function must run in the main thread to protect against
+ * concurrent read in job_finish_sync_locked(), takes the job_mutex
+ * lock to protect against the read in job_do_yield_locked(), and must
+ * be called when the coroutine is quiescent.
May be "job is quiscent" or "job is doing nothing", "no in-flight io operations in
job".
For example, backup has several running coroutines in contest of block_copy
process, and main coroutine of the job
is almost always "quescent"..
+ */
+void job_set_aio_context(Job *job, AioContext *ctx);
+
#endif
diff --git a/job.c b/job.c
index 85ae843f03..9f2fb2e73b 100644
--- a/job.c
+++ b/job.c
@@ -396,6 +396,17 @@ Job *job_get(const char *id)
return job_get_locked(id);
}
+void job_set_aio_context(Job *job, AioContext *ctx)
+{
+/* protect against read in job_finish_sync_locked and job_start */
+GLOBAL_STATE_CODE();
+/* protect against read in job_do_yield_locked */
+JOB_LOCK_GUARD();
+/* ensure the coroutine is quiescent while the AioContext is changed */
same not here.
+assert(job->paused || job_is_completed_locked(job));
+job->aio_context = ctx;
+}
+
/* Called with job_mutex *not* held. */
static void job_sleep_timer_cb(void *opaque)
{
@@ -1379,6 +1390,7 @@ int job_finish_sync_locked(Job *job,
{
Error *local_err = NULL;
int ret;
+GLOBAL_STATE_CODE();
[PULL 06/11] target/arm: Honour -semihosting-config userspace=on
From: Peter Maydell
Honour the commandline -semihosting-config userspace=on option,
instead of never permitting userspace semihosting calls in system
emulation mode, by passing the correct value to the is_userspace
argument of semihosting_enabled(), instead of manually checking and
always forbidding semihosting if the guest is in userspace and this
isn't the linux-user build.
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Message-Id: <20220822141230.3658237-3-peter.mayd...@linaro.org>
Signed-off-by: Richard Henderson
---
target/arm/translate-a64.c | 12 +---
target/arm/translate.c | 16
2 files changed, 5 insertions(+), 23 deletions(-)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 3decc8da57..9bed336b47 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -2219,17 +2219,7 @@ static void disas_exc(DisasContext *s, uint32_t insn)
* it is required for halting debug disabled: it will UNDEF.
* Secondly, "HLT 0xf000" is the A64 semihosting syscall instruction.
*/
-if (semihosting_enabled(false) && imm16 == 0xf000) {
-#ifndef CONFIG_USER_ONLY
-/* In system mode, don't allow userspace access to semihosting,
- * to provide some semblance of security (and for consistency
- * with our 32-bit semihosting).
- */
-if (s->current_el == 0) {
-unallocated_encoding(s);
-break;
-}
-#endif
+if (semihosting_enabled(s->current_el == 0) && imm16 == 0xf000) {
gen_exception_internal_insn(s, s->pc_curr, EXCP_SEMIHOST);
} else {
unallocated_encoding(s);
diff --git a/target/arm/translate.c b/target/arm/translate.c
index b1e013270d..5aaccbbf71 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1169,10 +1169,7 @@ static inline void gen_hlt(DisasContext *s, int imm)
* semihosting, to provide some semblance of security
* (and for consistency with our 32-bit semihosting).
*/
-if (semihosting_enabled(false) &&
-#ifndef CONFIG_USER_ONLY
-s->current_el != 0 &&
-#endif
+if (semihosting_enabled(s->current_el != 0) &&
(imm == (s->thumb ? 0x3c : 0xf000))) {
gen_exception_internal_insn(s, s->pc_curr, EXCP_SEMIHOST);
return;
@@ -6556,10 +6553,7 @@ static bool trans_BKPT(DisasContext *s, arg_BKPT *a)
/* BKPT is OK with ECI set and leaves it untouched */
s->eci_handled = true;
if (arm_dc_feature(s, ARM_FEATURE_M) &&
-semihosting_enabled(false) &&
-#ifndef CONFIG_USER_ONLY
-!IS_USER(s) &&
-#endif
+semihosting_enabled(s->current_el == 0) &&
(a->imm == 0xab)) {
gen_exception_internal_insn(s, s->pc_curr, EXCP_SEMIHOST);
} else {
@@ -8764,10 +8758,8 @@ static bool trans_SVC(DisasContext *s, arg_SVC *a)
{
const uint32_t semihost_imm = s->thumb ? 0xab : 0x123456;
-if (!arm_dc_feature(s, ARM_FEATURE_M) && semihosting_enabled(false) &&
-#ifndef CONFIG_USER_ONLY
-!IS_USER(s) &&
-#endif
+if (!arm_dc_feature(s, ARM_FEATURE_M) &&
+semihosting_enabled(s->current_el == 0) &&
(a->imm == semihost_imm)) {
gen_exception_internal_insn(s, s->pc_curr, EXCP_SEMIHOST);
} else {
--
2.34.1
[PULL 03/11] target/m68k: Use semihosting/syscalls.h
This separates guest file descriptors from host file descriptors,
and utilizes shared infrastructure for integration with gdbstub.
Acked-by: Laurent Vivier
Signed-off-by: Richard Henderson
---
target/m68k/m68k-semi.c | 281 +++-
1 file changed, 49 insertions(+), 232 deletions(-)
diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
index d0697ddbd1..586a801034 100644
--- a/target/m68k/m68k-semi.c
+++ b/target/m68k/m68k-semi.c
@@ -21,6 +21,7 @@
#include "cpu.h"
#include "exec/gdbstub.h"
+#include "semihosting/syscalls.h"
#include "semihosting/softmmu-uaccess.h"
#include "hw/boards.h"
#include "qemu/log.h"
@@ -40,56 +41,6 @@
#define HOSTED_ISATTY 12
#define HOSTED_SYSTEM 13
-static int translate_openflags(int flags)
-{
-int hf;
-
-if (flags & GDB_O_WRONLY)
-hf = O_WRONLY;
-else if (flags & GDB_O_RDWR)
-hf = O_RDWR;
-else
-hf = O_RDONLY;
-
-if (flags & GDB_O_APPEND) hf |= O_APPEND;
-if (flags & GDB_O_CREAT) hf |= O_CREAT;
-if (flags & GDB_O_TRUNC) hf |= O_TRUNC;
-if (flags & GDB_O_EXCL) hf |= O_EXCL;
-
-return hf;
-}
-
-static void translate_stat(CPUM68KState *env, target_ulong addr, struct stat
*s)
-{
-struct gdb_stat *p;
-
-p = lock_user(VERIFY_WRITE, addr, sizeof(struct gdb_stat), 0);
-if (!p) {
-/* FIXME - should this return an error code? */
-return;
-}
-p->gdb_st_dev = cpu_to_be32(s->st_dev);
-p->gdb_st_ino = cpu_to_be32(s->st_ino);
-p->gdb_st_mode = cpu_to_be32(s->st_mode);
-p->gdb_st_nlink = cpu_to_be32(s->st_nlink);
-p->gdb_st_uid = cpu_to_be32(s->st_uid);
-p->gdb_st_gid = cpu_to_be32(s->st_gid);
-p->gdb_st_rdev = cpu_to_be32(s->st_rdev);
-p->gdb_st_size = cpu_to_be64(s->st_size);
-#ifdef _WIN32
-/* Windows stat is missing some fields. */
-p->gdb_st_blksize = 0;
-p->gdb_st_blocks = 0;
-#else
-p->gdb_st_blksize = cpu_to_be64(s->st_blksize);
-p->gdb_st_blocks = cpu_to_be64(s->st_blocks);
-#endif
-p->gdb_st_atime = cpu_to_be32(s->st_atime);
-p->gdb_st_mtime = cpu_to_be32(s->st_mtime);
-p->gdb_st_ctime = cpu_to_be32(s->st_ctime);
-unlock_user(p, addr, sizeof(struct gdb_stat));
-}
-
static void m68k_semi_u32_cb(CPUState *cs, uint64_t ret, int err)
{
M68kCPU *cpu = M68K_CPU(cs);
@@ -129,248 +80,109 @@ static void m68k_semi_u64_cb(CPUState *cs, uint64_t ret,
int err)
*/
#define GET_ARG(n) do { \
if (get_user_ual(arg ## n, args + (n) * 4)) { \
-result = -1;\
-errno = EFAULT; \
goto failed;\
} \
} while (0)
+#define GET_ARG64(n) do { \
+if (get_user_ual(arg ## n, args + (n) * 4)) { \
+goto failed64; \
+} \
+} while (0)
+
+
void do_m68k_semihosting(CPUM68KState *env, int nr)
{
CPUState *cs = env_cpu(env);
uint32_t args;
target_ulong arg0, arg1, arg2, arg3;
-void *p;
-void *q;
-uint32_t len;
-uint32_t result;
args = env->dregs[1];
switch (nr) {
case HOSTED_EXIT:
gdb_exit(env->dregs[0]);
exit(env->dregs[0]);
+
case HOSTED_OPEN:
GET_ARG(0);
GET_ARG(1);
GET_ARG(2);
GET_ARG(3);
-if (use_gdb_syscalls()) {
-gdb_do_syscall(m68k_semi_u32_cb, "open,%s,%x,%x", arg0, (int)arg1,
- arg2, arg3);
-return;
-} else {
-p = lock_user_string(arg0);
-if (!p) {
-/* FIXME - check error code? */
-result = -1;
-} else {
-result = open(p, translate_openflags(arg2), arg3);
-unlock_user(p, arg0, 0);
-}
-}
+semihost_sys_open(cs, m68k_semi_u32_cb, arg0, arg1, arg2, arg3);
break;
+
case HOSTED_CLOSE:
-{
-/* Ignore attempts to close stdin/out/err. */
-GET_ARG(0);
-int fd = arg0;
-if (fd > 2) {
-if (use_gdb_syscalls()) {
-gdb_do_syscall(m68k_semi_u32_cb, "close,%x", arg0);
-return;
-} else {
-result = close(fd);
-}
-} else {
-result = 0;
-}
-break;
-}
+GET_ARG(0);
+semihost_sys_close(cs, m68k_semi_u32_cb, arg0);
+break;
+
case HOSTED_READ:
GET_ARG(0);
GET_ARG(1);
GET_ARG(2);
-len = arg2;
-if (use_gdb_syscalls()) {
-gdb_do_syscall(m68k_semi_u32_cb, "read,%x,%x,%x",
- arg0, arg1, len);
-return;
-}
[PULL 09/11] target/nios2: Honour -semihosting-config userspace=on
From: Peter Maydell
Honour the commandline -semihosting-config userspace=on option,
instead of always permitting userspace semihosting calls in system
emulation mode, by passing the correct value to the is_userspace
argument of semihosting_enabled().
Note that this is a behaviour change: if the user wants to
do semihosting calls from userspace they must now specifically
enable them on the command line.
nios2 semihosting is not implemented for linux-user builds.
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Message-Id: <20220822141230.3658237-6-peter.mayd...@linaro.org>
Signed-off-by: Richard Henderson
---
target/nios2/translate.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index ff631a42f6..8dc0a32c6c 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -817,8 +817,9 @@ static void gen_break(DisasContext *dc, uint32_t code,
uint32_t flags)
{
#ifndef CONFIG_USER_ONLY
/* The semihosting instruction is "break 1". */
+bool is_user = FIELD_EX32(dc->tb_flags, TBFLAGS, U);
R_TYPE(instr, code);
-if (semihosting_enabled(false) && instr.imm5 == 1) {
+if (semihosting_enabled(is_user) && instr.imm5 == 1) {
t_gen_helper_raise_exception(dc, EXCP_SEMIHOST);
return;
}
--
2.34.1
[PULL 00/11] semihosting patch queue
The following changes since commit 79dfa177ae348bb5ab5f97c0915359b13d6186e2: Merge tag 'pull-qapi-2022-09-07' of git://repo.or.cz/qemu/armbru into staging (2022-09-07 13:13:30 -0400) are available in the Git repository at: https://gitlab.com/rth7680/qemu.git tags/pull-semi-20220914 for you to fetch changes up to 7d7fb11615809839ff858328134c6a0abad27ea4: target/riscv: Honour -semihosting-config userspace=on and enable=on (2022-09-13 17:18:21 +0100) Convert m68k to semihosting/syscalls.h. Convert nios2 to semihosting/syscalls.h. Allow optional use of semihosting from userspace. Peter Maydell (7): semihosting: Allow optional use of semihosting from userspace target/arm: Honour -semihosting-config userspace=on target/m68k: Honour -semihosting-config userspace=on target/mips: Honour -semihosting-config userspace=on target/nios2: Honour -semihosting-config userspace=on target/xtensa: Honour -semihosting-config userspace=on target/riscv: Honour -semihosting-config userspace=on and enable=on Richard Henderson (4): target/nios2: Use semihosting/syscalls.h target/nios2: Convert semihosting errno to gdb remote errno target/m68k: Use semihosting/syscalls.h target/m68k: Convert semihosting errno to gdb remote errno include/semihosting/semihost.h | 10 +- semihosting/config.c | 10 +- softmmu/vl.c | 2 +- stubs/semihost.c | 2 +- target/arm/translate-a64.c | 12 +- target/arm/translate.c | 16 +- target/m68k/m68k-semi.c| 306 ++- target/m68k/op_helper.c| 3 +- target/mips/tcg/translate.c| 9 +- target/nios2/nios2-semi.c | 321 ++--- target/nios2/translate.c | 3 +- target/riscv/cpu_helper.c | 9 +- target/riscv/translate.c | 1 + target/xtensa/translate.c | 7 +- target/mips/tcg/micromips_translate.c.inc | 6 +- target/mips/tcg/mips16e_translate.c.inc| 2 +- target/mips/tcg/nanomips_translate.c.inc | 4 +- target/riscv/insn_trans/trans_privileged.c.inc | 3 +- qemu-options.hx| 11 +- 19 files changed, 209 insertions(+), 528 deletions(-)
[PULL 02/11] target/nios2: Convert semihosting errno to gdb remote errno
The semihosting abi used by nios2 uses the gdb remote
protocol filesys errnos.
Signed-off-by: Richard Henderson
---
target/nios2/nios2-semi.c | 33 +++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/target/nios2/nios2-semi.c b/target/nios2/nios2-semi.c
index 614fd76695..f76e8588c5 100644
--- a/target/nios2/nios2-semi.c
+++ b/target/nios2/nios2-semi.c
@@ -43,6 +43,35 @@
#define HOSTED_ISATTY 12
#define HOSTED_SYSTEM 13
+static int host_to_gdb_errno(int err)
+{
+#define E(X) case E##X: return GDB_E##X
+switch (err) {
+E(PERM);
+E(NOENT);
+E(INTR);
+E(BADF);
+E(ACCES);
+E(FAULT);
+E(BUSY);
+E(EXIST);
+E(NODEV);
+E(NOTDIR);
+E(ISDIR);
+E(INVAL);
+E(NFILE);
+E(MFILE);
+E(FBIG);
+E(NOSPC);
+E(SPIPE);
+E(ROFS);
+E(NAMETOOLONG);
+default:
+return GDB_EUNKNOWN;
+}
+#undef E
+}
+
static void nios2_semi_u32_cb(CPUState *cs, uint64_t ret, int err)
{
Nios2CPU *cpu = NIOS2_CPU(cs);
@@ -50,7 +79,7 @@ static void nios2_semi_u32_cb(CPUState *cs, uint64_t ret, int
err)
target_ulong args = env->regs[R_ARG1];
if (put_user_u32(ret, args) ||
-put_user_u32(err, args + 4)) {
+put_user_u32(host_to_gdb_errno(err), args + 4)) {
/*
* The nios2 semihosting ABI does not provide any way to report this
* error to the guest, so the best we can do is log it in qemu.
@@ -69,7 +98,7 @@ static void nios2_semi_u64_cb(CPUState *cs, uint64_t ret, int
err)
if (put_user_u32(ret >> 32, args) ||
put_user_u32(ret, args + 4) ||
-put_user_u32(err, args + 8)) {
+put_user_u32(host_to_gdb_errno(err), args + 8)) {
/* No way to report this via nios2 semihosting ABI; just log it */
qemu_log_mask(LOG_GUEST_ERROR, "nios2-semihosting: return value "
"discarded because argument block not writable\n");
--
2.34.1
Re: [PATCH v11 17/21] job.h: categorize JobDriver callbacks that need the AioContext lock
On 8/26/22 16:21, Emanuele Giuseppe Esposito wrote: Some callbacks implementation use bdrv_* APIs that assume the AioContext lock is held. Make sure this invariant is documented. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
[PATCH v2 4/4] tests/docker: run script use realpath instead of readlink
From: "Lucas Mateus Castro (alqotel)" The alpine docker image only comes with busybox, which doesn't have the '-e' option on its readlink, so change it to 'realpath' to avoid that problem. Suggested-by: Daniel P. Berrangé Signed-off-by: Lucas Mateus Castro (alqotel) --- tests/docker/run | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/docker/run b/tests/docker/run index 421393046b..9eb96129da 100755 --- a/tests/docker/run +++ b/tests/docker/run @@ -15,7 +15,7 @@ if test -n "$V"; then set -x fi -BASE="$(dirname $(readlink -e $0))" +BASE="$(dirname $(realpath $0))" # Prepare the environment export PATH=/usr/lib/ccache:/usr/lib64/ccache:$PATH -- 2.31.1
Re: [PATCH v11 16/21] blockjob: protect iostatus field in BlockJob struct
On 8/26/22 16:20, Emanuele Giuseppe Esposito wrote: iostatus is the only field (together with .job) that needs protection using the job mutex. It is set in the main loop (GLOBAL_STATE functions) but read in I/O code (block_job_error_action). In order to protect it, change block_job_iostatus_set_err to block_job_iostatus_set_err_locked(), always called under job lock. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
Re: [PATCH 2/3] vdpa: load vlan configuration at NIC startup
On Wed, Sep 14, 2022 at 1:33 PM Si-Wei Liu wrote:
>
>
>
> On 9/14/2022 3:20 AM, Jason Wang wrote:
> > On Fri, Sep 9, 2022 at 4:02 PM Eugenio Perez Martin
> > wrote:
> >> On Fri, Sep 9, 2022 at 8:40 AM Jason Wang wrote:
> >>> On Fri, Sep 9, 2022 at 2:38 PM Jason Wang wrote:
> On Wed, Sep 7, 2022 at 12:36 AM Eugenio Pérez
> wrote:
> > To have enabled vlans at device startup may happen in the destination of
> > a live migration, so this configuration must be restored.
> >
> > At this moment the code is not accessible, since SVQ refuses to start if
> > vlan feature is exposed by the device.
> >
> > Signed-off-by: Eugenio Pérez
> > ---
> > net/vhost-vdpa.c | 46 --
> > 1 file changed, 44 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > index 4bc3fd01a8..ecbfd08eb9 100644
> > --- a/net/vhost-vdpa.c
> > +++ b/net/vhost-vdpa.c
> > @@ -100,6 +100,8 @@ static const uint64_t vdpa_svq_device_features =
> > BIT_ULL(VIRTIO_NET_F_RSC_EXT) |
> > BIT_ULL(VIRTIO_NET_F_STANDBY);
> >
> > +#define MAX_VLAN(1 << 12) /* Per 802.1Q definition */
> > +
> > VHostNetState *vhost_vdpa_get_vhost_net(NetClientState *nc)
> > {
> > VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> > @@ -423,6 +425,47 @@ static int vhost_vdpa_net_load_mq(VhostVDPAState
> > *s,
> > return *s->status != VIRTIO_NET_OK;
> > }
> >
> > +static int vhost_vdpa_net_load_single_vlan(VhostVDPAState *s,
> > + const VirtIONet *n,
> > + uint16_t vid)
> > +{
> > +ssize_t dev_written = vhost_vdpa_net_load_cmd(s,
> > VIRTIO_NET_CTRL_VLAN,
> > +
> > VIRTIO_NET_CTRL_VLAN_ADD,
> > + , sizeof(vid));
> > +if (unlikely(dev_written < 0)) {
> > +return dev_written;
> > +}
> > +
> > +if (unlikely(*s->status != VIRTIO_NET_OK)) {
> > +return -EINVAL;
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static int vhost_vdpa_net_load_vlan(VhostVDPAState *s,
> > +const VirtIONet *n)
> > +{
> > +uint64_t features = n->parent_obj.guest_features;
> > +
> > +if (!(features & BIT_ULL(VIRTIO_NET_F_CTRL_VLAN))) {
> > +return 0;
> > +}
> > +
> > +for (int i = 0; i < MAX_VLAN >> 5; i++) {
> > +for (int j = 0; n->vlans[i] && j <= 0x1f; j++) {
> > +if (n->vlans[i] & (1U << j)) {
> > +int r = vhost_vdpa_net_load_single_vlan(s, n, (i << 5)
> > + j);
> This seems to cause a lot of latency if the driver has a lot of vlans.
>
> I wonder if it's simply to let all vlan traffic go by disabling
> CTRL_VLAN feature at vDPA layer.
> >> The guest will not be able to recover that vlan configuration.
> > Spec said it's best effort and we actually don't do this for
> > vhost-net/user for years and manage to survive.
> I thought there's a border line between best effort and do nothing. ;-)
>
I also think it is worth at least trying to migrate it for sure. For
MAC there may be too many entries, but VLAN should be totally
manageable (and the information is already there, the bitmap is
actually being migrated).
> I think that the reason this could survive is really software
> implementation specific - say if the backend doesn't start with VLAN
> filtering disabled (meaning allow all VLAN traffic to pass) then it
> would become a problem. This won't be a problem for almost PF NICs but
> may be problematic for vDPA e.g. VF backed VDPAs.
I'd say the driver would expect all vlan filters to be cleared after a
reset, isn't it? The spec doesn't explicitly say anything about that
as far as I see.
> >
> >>> Another idea is to extend the spec to allow us to accept a bitmap of
> >>> the vlan ids via a single command, then we will be fine.
> >>>
> >> I'm not sure if adding more ways to configure something is the answer,
> >> but I'd be ok to implement it.
> >>
> >> Another idea is to allow the sending of many CVQ commands in parallel.
> >> It shouldn't be very hard to achieve using exposed buffers as ring
> >> buffers, and it will short down the start of the devices with many
> >> features.
> > In the extreme case, what if guests decide to filter all of the vlans?
> > It means we need MAX_VLAN commands which may exceeds the size of the
> > control virtqueue.
> Indeed, that's a case where a single flat device state blob would be
> more efficient for hardware drivers to apply than individual control
> command messages do.
>
If we're going that route, being able to set a bitmap for vlan
filtering
Re: [PATCH v11 11/21] jobs: group together API calls under the same job lock
On 8/26/22 16:20, Emanuele Giuseppe Esposito wrote:
Now that the API offers also _locked() functions, take advantage
of it and give also the caller control to take the lock and call
_locked functions.
This makes sense especially when we have for loops, because it
makes no sense to have:
for(job = job_next(); ...)
where each job_next() takes the lock internally.
Instead we want
JOB_LOCK_GUARD();
for(job = job_next_locked(); ...)
In addition, protect also direct field accesses, by either creating a
new critical section or widening the existing ones.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Signed-off-by: Emanuele Giuseppe Esposito
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
block.c| 17 ++---
blockdev.c | 14 ++
blockjob.c | 35 ++-
job-qmp.c | 9 ++---
job.c | 7 +--
monitor/qmp-cmds.c | 7 +--
qemu-img.c | 17 +++--
7 files changed, 69 insertions(+), 37 deletions(-)
[..]
diff --git a/job.c b/job.c
index dcd561fd46..e336af0c1c 100644
--- a/job.c
+++ b/job.c
job.c hunks are unrelated in this patch, they should go to patch 05.
@@ -672,7 +672,7 @@ void coroutine_fn job_pause_point(Job *job)
job_pause_point_locked(job);
}
-void job_yield_locked(Job *job)
+static void job_yield_locked(Job *job)
{
assert(job->busy);
@@ -1046,11 +1046,14 @@ static void job_completed_txn_abort_locked(Job *job)
/* Called with job_mutex held, but releases it temporarily */
static int job_prepare_locked(Job *job)
{
+int ret;
+
GLOBAL_STATE_CODE();
if (job->ret == 0 && job->driver->prepare) {
job_unlock();
-job->ret = job->driver->prepare(job);
+ret = job->driver->prepare(job);
job_lock();
+job->ret = ret;
job_update_rc_locked(job);
}
return job->ret;
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 7314cd813d..81c8fdadf8 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -135,8 +135,11 @@ void qmp_cont(Error **errp)
blk_iostatus_reset(blk);
}
-for (job = block_job_next(NULL); job; job = block_job_next(job)) {
-block_job_iostatus_reset(job);
+WITH_JOB_LOCK_GUARD() {
+for (job = block_job_next_locked(NULL); job;
+ job = block_job_next_locked(job)) {
+block_job_iostatus_reset_locked(job);
+}
}
/* Continuing after completed migration. Images have been inactivated to
diff --git a/qemu-img.c b/qemu-img.c
index 7d4b33b3da..c8ae704166 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -912,25 +912,30 @@ static void run_block_job(BlockJob *job, Error **errp)
int ret = 0;
aio_context_acquire(aio_context);
-job_ref(>job);
+job_lock();
+job_ref_locked(>job);
do {
float progress = 0.0f;
+job_unlock();
aio_poll(aio_context, true);
progress_get_snapshot(>job.progress, _current,
- _total);
+_total);
broken indentation
if (progress_total) {
progress = (float)progress_current / progress_total * 100.f;
}
qemu_progress_print(progress, 0);
-} while (!job_is_ready(>job) && !job_is_completed(>job));
+job_lock();
+} while (!job_is_ready_locked(>job) &&
+!job_is_completed_locked(>job));
and here
-if (!job_is_completed(>job)) {
-ret = job_complete_sync(>job, errp);
+if (!job_is_completed_locked(>job)) {
+ret = job_complete_sync_locked(>job, errp);
} else {
ret = job->job.ret;
}
-job_unref(>job);
+job_unref_locked(>job);
+job_unlock();
aio_context_release(aio_context);
/* publish completion progress only when success */
--
Best regards,
Vladimir
[PULL 10/11] target/xtensa: Honour -semihosting-config userspace=on
From: Peter Maydell
Honour the commandline -semihosting-config userspace=on option,
instead of always permitting userspace semihosting calls in system
emulation mode, by passing the correct value to the is_userspace
argument of semihosting_enabled().
Note that this is a behaviour change: if the user wants to
do semihosting calls from userspace they must now specifically
enable them on the command line.
xtensa semihosting is not implemented for linux-user builds.
Signed-off-by: Peter Maydell
Acked-by: Max Filippov
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20220822141230.3658237-7-peter.mayd...@linaro.org>
Signed-off-by: Richard Henderson
---
target/xtensa/translate.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index afae8a1bea..bdd4690a5c 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -2362,13 +2362,14 @@ static uint32_t test_exceptions_simcall(DisasContext
*dc,
const OpcodeArg arg[],
const uint32_t par[])
{
+bool is_semi = semihosting_enabled(dc->cring != 0);
#ifdef CONFIG_USER_ONLY
bool ill = true;
#else
/* Between RE.2 and RE.3 simcall opcode's become nop for the hardware. */
-bool ill = dc->config->hw_version <= 250002 && !semihosting_enabled(false);
+bool ill = dc->config->hw_version <= 250002 && !is_semi;
#endif
-if (ill || !semihosting_enabled(false)) {
+if (ill || !is_semi) {
qemu_log_mask(LOG_GUEST_ERROR, "SIMCALL but semihosting is
disabled\n");
}
return ill ? XTENSA_OP_ILL : 0;
@@ -2378,7 +2379,7 @@ static void translate_simcall(DisasContext *dc, const
OpcodeArg arg[],
const uint32_t par[])
{
#ifndef CONFIG_USER_ONLY
-if (semihosting_enabled(false)) {
+if (semihosting_enabled(dc->cring != 0)) {
gen_helper_simcall(cpu_env);
}
#endif
--
2.34.1
[PATCH v2 2/4] scripts/ci/setup: Fix libxen requirements
From: "Lucas Mateus Castro (alqotel)" XEN hypervisor is only available in ARM and x86, but the yaml only checked if the architecture is different from s390x, changed it to a more accurate test. Tested this change on a Ubuntu 20.04 ppc64le. Signed-off-by: Lucas Mateus Castro (alqotel) Reviewed-by: Alex Bennée --- scripts/ci/setup/build-environment.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml index 6df3e61d94..7535228685 100644 --- a/scripts/ci/setup/build-environment.yml +++ b/scripts/ci/setup/build-environment.yml @@ -97,7 +97,7 @@ state: present when: - ansible_facts['distribution'] == 'Ubuntu' -- ansible_facts['architecture'] != 's390x' +- ansible_facts['architecture'] == 'aarch64' or ansible_facts['architecture'] == 'x86_64' - name: Install basic packages to build QEMU on Ubuntu 20.04 package: -- 2.31.1
Proposal for a fixed ram migration stream format for file-based migrations
Hello, Based on several discussions I've had in the past 2 days and time spent looking at the migration stream code I came up with the following proposal for changes to the stream format. Let me recap what we have right now: ...() Where is put only when the current page we are writing to belongs to a different block than the last written page. My proposal is to change this format to: ... Each page belonging to a ramblock would be at a fixed offset in the file. Which is going to be + page_offset. The size of the region for a particular ramblock would be ramblock::used_length but not the whole range is going to be written, only those pages which have been dirtied. If we assume a 1tb ramblock then we'd make the the region 1 tb but it will be sparsely populated. This means that we can have the following layout for the memory range: |---p---ppppp---p| 01tb Each 'p' signifies an allocated page and '-' is a page which hasn't been dirtied ergo its index in the stream is not touched. This of course would result in having a lot of holes, so when the incoming migration starts parsing the stream it might end up in a situation where page at offset 0 is read, but then it has to jump some at arbitrary location for the next page. To avoid resorting to calling into the filesystem and dealing with fiemap's format I also intend to extend the ramblock header format by adding a dirty bitmap in which every bit would signify whether a page has been written or not, the bit's position would be used to index into the allocated space for pages. Simple maths shows that for 4k pages, 1tb can be indexed with just 32kbytes of memory. The way the indexing would work is page = ramblock->offset (this is the offset in the file, not the in-memory ::ofset field) + bit's position * target_page_size That way we can still handle sparsely dirtied memory and have direct mapping in the file, this dirty bitmap would be populated during the course of ram migration and once the final page is written, those in-memory bitmaps would be written into their respective positions in the file, needless to say this would be supported only on seekable channels (for now only file-based channels). Another important thing worth mentioning is I don't intend on incrementing the format version but rather introducing a new migration capability. Any thoughts/comments/suggestions, would be highly appreciated.
[PATCH v2 1/4] scripts/ci/setup: ninja missing from build-environment
From: "Lucas Mateus Castro (alqotel)" ninja-build is missing from the RHEL environment, so a system prepared with that script would still fail to compile QEMU. Tested on a Fedora 36 Signed-off-by: Lucas Mateus Castro (alqotel) --- scripts/ci/setup/build-environment.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml index 232525b91d..6df3e61d94 100644 --- a/scripts/ci/setup/build-environment.yml +++ b/scripts/ci/setup/build-environment.yml @@ -150,6 +150,7 @@ - libepoxy-devel - libgcrypt-devel - lzo-devel + - ninja-build - make - mesa-libEGL-devel - nettle-devel -- 2.31.1
[PULL 07/11] target/m68k: Honour -semihosting-config userspace=on
From: Peter Maydell
Honour the commandline -semihosting-config userspace=on option,
instead of never permitting userspace semihosting calls in system
emulation mode, by passing the correct value to the is_userspace
argument of semihosting_enabled(), instead of manually checking and
always forbidding semihosting if the guest is in userspace.
(Note that target/m68k doesn't support semihosting at all
in the linux-user build.)
Signed-off-by: Peter Maydell
Reviewed-by: Laurent Vivier
Reviewed-by: Richard Henderson
Message-Id: <20220822141230.3658237-4-peter.mayd...@linaro.org>
Signed-off-by: Richard Henderson
---
target/m68k/op_helper.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 4b3dfec130..a96a034050 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -203,8 +203,7 @@ static void cf_interrupt_all(CPUM68KState *env, int is_hw)
cf_rte(env);
return;
case EXCP_HALT_INSN:
-if (semihosting_enabled(false)
-&& (env->sr & SR_S) != 0
+if (semihosting_enabled((env->sr & SR_S) == 0)
&& (env->pc & 3) == 0
&& cpu_lduw_code(env, env->pc - 4) == 0x4e71
&& cpu_ldl_code(env, env->pc) == 0x4e7bf000) {
--
2.34.1
[PULL 08/11] target/mips: Honour -semihosting-config userspace=on
From: Peter Maydell
Honour the commandline -semihosting-config userspace=on option,
instead of always permitting userspace semihosting calls in system
emulation mode, by passing the correct value to the is_userspace
argument of semihosting_enabled().
Note that this is a behaviour change: if the user wants to
do semihosting calls from userspace they must now specifically
enable them on the command line.
MIPS semihosting is not implemented for linux-user builds.
Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20220822141230.3658237-5-peter.mayd...@linaro.org>
Signed-off-by: Richard Henderson
---
target/mips/tcg/translate.c | 9 +
target/mips/tcg/micromips_translate.c.inc | 6 +++---
target/mips/tcg/mips16e_translate.c.inc | 2 +-
target/mips/tcg/nanomips_translate.c.inc | 4 ++--
4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 0d936e2648..c3f92ea652 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -12082,12 +12082,13 @@ static void gen_cache_operation(DisasContext *ctx,
uint32_t op, int base,
tcg_temp_free_i32(t0);
}
-static inline bool is_uhi(int sdbbp_code)
+static inline bool is_uhi(DisasContext *ctx, int sdbbp_code)
{
#ifdef CONFIG_USER_ONLY
return false;
#else
-return semihosting_enabled() && sdbbp_code == 1;
+bool is_user = (ctx->hflags & MIPS_HFLAG_KSU) == MIPS_HFLAG_UM;
+return semihosting_enabled(is_user) && sdbbp_code == 1;
#endif
}
@@ -13898,7 +13899,7 @@ static void decode_opc_special_r6(CPUMIPSState *env,
DisasContext *ctx)
}
break;
case R6_OPC_SDBBP:
-if (is_uhi(extract32(ctx->opcode, 6, 20))) {
+if (is_uhi(ctx, extract32(ctx->opcode, 6, 20))) {
ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
if (ctx->hflags & MIPS_HFLAG_SBRI) {
@@ -14310,7 +14311,7 @@ static void decode_opc_special2_legacy(CPUMIPSState
*env, DisasContext *ctx)
gen_cl(ctx, op1, rd, rs);
break;
case OPC_SDBBP:
-if (is_uhi(extract32(ctx->opcode, 6, 20))) {
+if (is_uhi(ctx, extract32(ctx->opcode, 6, 20))) {
ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
/*
diff --git a/target/mips/tcg/micromips_translate.c.inc
b/target/mips/tcg/micromips_translate.c.inc
index b2c696f891..632895cc9e 100644
--- a/target/mips/tcg/micromips_translate.c.inc
+++ b/target/mips/tcg/micromips_translate.c.inc
@@ -825,7 +825,7 @@ static void gen_pool16c_insn(DisasContext *ctx)
generate_exception_break(ctx, extract32(ctx->opcode, 0, 4));
break;
case SDBBP16:
-if (is_uhi(extract32(ctx->opcode, 0, 4))) {
+if (is_uhi(ctx, extract32(ctx->opcode, 0, 4))) {
ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
/*
@@ -941,7 +941,7 @@ static void gen_pool16c_r6_insn(DisasContext *ctx)
break;
case R6_SDBBP16:
/* SDBBP16 */
-if (is_uhi(extract32(ctx->opcode, 6, 4))) {
+if (is_uhi(ctx, extract32(ctx->opcode, 6, 4))) {
ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
if (ctx->hflags & MIPS_HFLAG_SBRI) {
@@ -1310,7 +1310,7 @@ static void gen_pool32axf(CPUMIPSState *env, DisasContext
*ctx, int rt, int rs)
generate_exception_end(ctx, EXCP_SYSCALL);
break;
case SDBBP:
-if (is_uhi(extract32(ctx->opcode, 16, 10))) {
+if (is_uhi(ctx, extract32(ctx->opcode, 16, 10))) {
ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
check_insn(ctx, ISA_MIPS_R1);
diff --git a/target/mips/tcg/mips16e_translate.c.inc
b/target/mips/tcg/mips16e_translate.c.inc
index 7568933e23..918b15d55c 100644
--- a/target/mips/tcg/mips16e_translate.c.inc
+++ b/target/mips/tcg/mips16e_translate.c.inc
@@ -951,7 +951,7 @@ static int decode_ase_mips16e(CPUMIPSState *env,
DisasContext *ctx)
}
break;
case RR_SDBBP:
-if (is_uhi(extract32(ctx->opcode, 5, 6))) {
+if (is_uhi(ctx, extract32(ctx->opcode, 5, 6))) {
ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
/*
diff --git a/target/mips/tcg/nanomips_translate.c.inc
b/target/mips/tcg/nanomips_translate.c.inc
index b3aff22c18..812c111e3c 100644
--- a/target/mips/tcg/nanomips_translate.c.inc
+++ b/target/mips/tcg/nanomips_translate.c.inc
@@ -3694,7 +3694,7 @@ static int decode_nanomips_32_48_opc(CPUMIPSState *env,
DisasContext *ctx)
generate_exception_end(ctx, EXCP_BREAK);
break;
case NM_SDBBP:
-if (is_uhi(extract32(ctx->opcode, 0, 19))) {
+if (is_uhi(ctx, extract32(ctx->opcode, 0, 19))) {
ctx->base.is_jmp = DISAS_SEMIHOST;
[PULL 05/11] semihosting: Allow optional use of semihosting from userspace
From: Peter Maydell
Currently our semihosting implementations generally prohibit use of
semihosting calls in system emulation from the guest userspace. This
is a very long standing behaviour justified originally "to provide
some semblance of security" (since code with access to the
semihosting ABI can do things like read and write arbitrary files on
the host system). However, it is sometimes useful to be able to run
trusted guest code which performs semihosting calls from guest
userspace, notably for test code. Add a command line suboption to
the existing semihosting-config option group so that you can
explicitly opt in to semihosting from guest userspace with
-semihosting-config userspace=on
(There is no equivalent option for the user-mode emulator, because
there by definition all code runs in userspace and has access to
semihosting already.)
This commit adds the infrastructure for the command line option and
adds a bool 'is_user' parameter to the function
semihosting_userspace_enabled() that target code can use to check
whether it should be permitting the semihosting call for userspace.
It mechanically makes all the callsites pass 'false', so they
continue checking "is semihosting enabled in general". Subsequent
commits will make each target that implements semihosting honour the
userspace=on option by passing the correct value and removing
whatever "don't do this for userspace" checking they were doing by
hand.
Signed-off-by: Peter Maydell
Acked-by: Alex Bennée
Reviewed-by: Alistair Francis
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20220822141230.3658237-2-peter.mayd...@linaro.org>
Signed-off-by: Richard Henderson
---
include/semihosting/semihost.h | 10 --
semihosting/config.c | 10 --
softmmu/vl.c | 2 +-
stubs/semihost.c | 2 +-
target/arm/translate-a64.c | 2 +-
target/arm/translate.c | 6 +++---
target/m68k/op_helper.c| 2 +-
target/nios2/translate.c | 2 +-
target/xtensa/translate.c | 6 +++---
qemu-options.hx| 11 +--
10 files changed, 36 insertions(+), 17 deletions(-)
diff --git a/include/semihosting/semihost.h b/include/semihosting/semihost.h
index 93a3c21b44..efd2efa25a 100644
--- a/include/semihosting/semihost.h
+++ b/include/semihosting/semihost.h
@@ -27,7 +27,7 @@ typedef enum SemihostingTarget {
} SemihostingTarget;
#ifdef CONFIG_USER_ONLY
-static inline bool semihosting_enabled(void)
+static inline bool semihosting_enabled(bool is_user)
{
return true;
}
@@ -52,7 +52,13 @@ static inline const char *semihosting_get_cmdline(void)
return NULL;
}
#else /* !CONFIG_USER_ONLY */
-bool semihosting_enabled(void);
+/**
+ * semihosting_enabled:
+ * @is_user: true if guest code is in usermode (i.e. not privileged)
+ *
+ * Return true if guest code is allowed to make semihosting calls.
+ */
+bool semihosting_enabled(bool is_user);
SemihostingTarget semihosting_get_target(void);
const char *semihosting_get_arg(int i);
int semihosting_get_argc(void);
diff --git a/semihosting/config.c b/semihosting/config.c
index e171d4d6bc..89a1759687 100644
--- a/semihosting/config.c
+++ b/semihosting/config.c
@@ -34,6 +34,9 @@ QemuOptsList qemu_semihosting_config_opts = {
{
.name = "enable",
.type = QEMU_OPT_BOOL,
+}, {
+.name = "userspace",
+.type = QEMU_OPT_BOOL,
}, {
.name = "target",
.type = QEMU_OPT_STRING,
@@ -50,6 +53,7 @@ QemuOptsList qemu_semihosting_config_opts = {
typedef struct SemihostingConfig {
bool enabled;
+bool userspace_enabled;
SemihostingTarget target;
char **argv;
int argc;
@@ -59,9 +63,9 @@ typedef struct SemihostingConfig {
static SemihostingConfig semihosting;
static const char *semihost_chardev;
-bool semihosting_enabled(void)
+bool semihosting_enabled(bool is_user)
{
-return semihosting.enabled;
+return semihosting.enabled && (!is_user || semihosting.userspace_enabled);
}
SemihostingTarget semihosting_get_target(void)
@@ -137,6 +141,8 @@ int qemu_semihosting_config_options(const char *optarg)
if (opts != NULL) {
semihosting.enabled = qemu_opt_get_bool(opts, "enable",
true);
+semihosting.userspace_enabled = qemu_opt_get_bool(opts, "userspace",
+ false);
const char *target = qemu_opt_get(opts, "target");
/* setup of chardev is deferred until they are initialised */
semihost_chardev = qemu_opt_get(opts, "chardev");
diff --git a/softmmu/vl.c b/softmmu/vl.c
index dea4005e47..263f029a8e 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1822,7 +1822,7 @@ static void qemu_apply_machine_options(QDict *qdict)
{
object_set_properties_from_keyval(OBJECT(current_machine), qdict, false,
_fatal);
-if
[PATCH v2 0/4] Patch series to set up a ppc64le CI
From: "Lucas Mateus Castro (alqotel)" This patch series aim to make easier to set up a compilation and CI environment on PPC64 and PPC64LE machines. v2: This patch series are only patches 2-4 of v1 and an alternative to patch 1 suggested by Daniel. Lucas Mateus Castro (alqotel) (4): scripts/ci/setup: ninja missing from build-environment scripts/ci/setup: Fix libxen requirements scripts/ci/setup: spice-server only on x86 aarch64 tests/docker: run script use realpath instead of readlink scripts/ci/setup/build-environment.yml | 15 +-- tests/docker/run | 2 +- 2 files changed, 14 insertions(+), 3 deletions(-) -- 2.31.1
[PULL 04/11] target/m68k: Convert semihosting errno to gdb remote errno
The semihosting abi used by m68k uses the gdb remote
protocol filesys errnos.
Acked-by: Laurent Vivier
Signed-off-by: Richard Henderson
---
target/m68k/m68k-semi.c | 33 +++--
1 file changed, 31 insertions(+), 2 deletions(-)
diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
index 586a801034..87b1314925 100644
--- a/target/m68k/m68k-semi.c
+++ b/target/m68k/m68k-semi.c
@@ -41,6 +41,35 @@
#define HOSTED_ISATTY 12
#define HOSTED_SYSTEM 13
+static int host_to_gdb_errno(int err)
+{
+#define E(X) case E##X: return GDB_E##X
+switch (err) {
+E(PERM);
+E(NOENT);
+E(INTR);
+E(BADF);
+E(ACCES);
+E(FAULT);
+E(BUSY);
+E(EXIST);
+E(NODEV);
+E(NOTDIR);
+E(ISDIR);
+E(INVAL);
+E(NFILE);
+E(MFILE);
+E(FBIG);
+E(NOSPC);
+E(SPIPE);
+E(ROFS);
+E(NAMETOOLONG);
+default:
+return GDB_EUNKNOWN;
+}
+#undef E
+}
+
static void m68k_semi_u32_cb(CPUState *cs, uint64_t ret, int err)
{
M68kCPU *cpu = M68K_CPU(cs);
@@ -48,7 +77,7 @@ static void m68k_semi_u32_cb(CPUState *cs, uint64_t ret, int
err)
target_ulong args = env->dregs[1];
if (put_user_u32(ret, args) ||
-put_user_u32(err, args + 4)) {
+put_user_u32(host_to_gdb_errno(err), args + 4)) {
/*
* The m68k semihosting ABI does not provide any way to report this
* error to the guest, so the best we can do is log it in qemu.
@@ -67,7 +96,7 @@ static void m68k_semi_u64_cb(CPUState *cs, uint64_t ret, int
err)
target_ulong args = env->dregs[1];
if (put_user_u32(ret >> 32, args) ||
put_user_u32(ret, args + 4) ||
-put_user_u32(err, args + 8)) {
+put_user_u32(host_to_gdb_errno(err), args + 8)) {
/* No way to report this via m68k semihosting ABI; just log it */
qemu_log_mask(LOG_GUEST_ERROR, "m68k-semihosting: return value "
"discarded because argument block not writable\n");
--
2.34.1
[PULL 01/11] target/nios2: Use semihosting/syscalls.h
This separates guest file descriptors from host file descriptors,
and utilizes shared infrastructure for integration with gdbstub.
Signed-off-by: Richard Henderson
---
target/nios2/nios2-semi.c | 296 +++---
1 file changed, 50 insertions(+), 246 deletions(-)
diff --git a/target/nios2/nios2-semi.c b/target/nios2/nios2-semi.c
index 55061bb2dc..614fd76695 100644
--- a/target/nios2/nios2-semi.c
+++ b/target/nios2/nios2-semi.c
@@ -24,6 +24,7 @@
#include "qemu/osdep.h"
#include "cpu.h"
#include "exec/gdbstub.h"
+#include "semihosting/syscalls.h"
#include "semihosting/softmmu-uaccess.h"
#include "qemu/log.h"
@@ -42,67 +43,6 @@
#define HOSTED_ISATTY 12
#define HOSTED_SYSTEM 13
-static int translate_openflags(int flags)
-{
-int hf;
-
-if (flags & GDB_O_WRONLY) {
-hf = O_WRONLY;
-} else if (flags & GDB_O_RDWR) {
-hf = O_RDWR;
-} else {
-hf = O_RDONLY;
-}
-
-if (flags & GDB_O_APPEND) {
-hf |= O_APPEND;
-}
-if (flags & GDB_O_CREAT) {
-hf |= O_CREAT;
-}
-if (flags & GDB_O_TRUNC) {
-hf |= O_TRUNC;
-}
-if (flags & GDB_O_EXCL) {
-hf |= O_EXCL;
-}
-
-return hf;
-}
-
-static bool translate_stat(CPUNios2State *env, target_ulong addr,
- struct stat *s)
-{
-struct gdb_stat *p;
-
-p = lock_user(VERIFY_WRITE, addr, sizeof(struct gdb_stat), 0);
-
-if (!p) {
-return false;
-}
-p->gdb_st_dev = cpu_to_be32(s->st_dev);
-p->gdb_st_ino = cpu_to_be32(s->st_ino);
-p->gdb_st_mode = cpu_to_be32(s->st_mode);
-p->gdb_st_nlink = cpu_to_be32(s->st_nlink);
-p->gdb_st_uid = cpu_to_be32(s->st_uid);
-p->gdb_st_gid = cpu_to_be32(s->st_gid);
-p->gdb_st_rdev = cpu_to_be32(s->st_rdev);
-p->gdb_st_size = cpu_to_be64(s->st_size);
-#ifdef _WIN32
-/* Windows stat is missing some fields. */
-p->gdb_st_blksize = 0;
-p->gdb_st_blocks = 0;
-#else
-p->gdb_st_blksize = cpu_to_be64(s->st_blksize);
-p->gdb_st_blocks = cpu_to_be64(s->st_blocks);
-#endif
-p->gdb_st_atime = cpu_to_be32(s->st_atime);
-p->gdb_st_mtime = cpu_to_be32(s->st_mtime);
-p->gdb_st_ctime = cpu_to_be32(s->st_ctime);
-unlock_user(p, addr, sizeof(struct gdb_stat));
-return true;
-}
-
static void nios2_semi_u32_cb(CPUState *cs, uint64_t ret, int err)
{
Nios2CPU *cpu = NIOS2_CPU(cs);
@@ -142,22 +82,22 @@ static void nios2_semi_u64_cb(CPUState *cs, uint64_t ret,
int err)
*/
#define GET_ARG(n) do { \
if (get_user_ual(arg ## n, args + (n) * 4)) { \
-result = -1;\
-errno = EFAULT; \
goto failed;\
} \
} while (0)
+#define GET_ARG64(n) do { \
+if (get_user_ual(arg ## n, args + (n) * 4)) { \
+goto failed64; \
+} \
+} while (0)
+
void do_nios2_semihosting(CPUNios2State *env)
{
CPUState *cs = env_cpu(env);
int nr;
uint32_t args;
target_ulong arg0, arg1, arg2, arg3;
-void *p;
-void *q;
-uint32_t len;
-uint32_t result;
nr = env->regs[R_ARG0];
args = env->regs[R_ARG1];
@@ -165,234 +105,98 @@ void do_nios2_semihosting(CPUNios2State *env)
case HOSTED_EXIT:
gdb_exit(env->regs[R_ARG0]);
exit(env->regs[R_ARG0]);
+
case HOSTED_OPEN:
GET_ARG(0);
GET_ARG(1);
GET_ARG(2);
GET_ARG(3);
-if (use_gdb_syscalls()) {
-gdb_do_syscall(nios2_semi_u32_cb, "open,%s,%x,%x", arg0, (int)arg1,
- arg2, arg3);
-return;
-} else {
-p = lock_user_string(arg0);
-if (!p) {
-result = -1;
-errno = EFAULT;
-} else {
-result = open(p, translate_openflags(arg2), arg3);
-unlock_user(p, arg0, 0);
-}
-}
+semihost_sys_open(cs, nios2_semi_u32_cb, arg0, arg1, arg2, arg3);
break;
+
case HOSTED_CLOSE:
-{
-/* Ignore attempts to close stdin/out/err. */
-GET_ARG(0);
-int fd = arg0;
-if (fd > 2) {
-if (use_gdb_syscalls()) {
-gdb_do_syscall(nios2_semi_u32_cb, "close,%x", arg0);
-return;
-} else {
-result = close(fd);
-}
-} else {
-result = 0;
-}
-break;
-}
+GET_ARG(0);
+semihost_sys_close(cs, nios2_semi_u32_cb, arg0);
+break;
+
case HOSTED_READ:
GET_ARG(0);
GET_ARG(1);
GET_ARG(2);
-len = arg2;
-if
Re: [PATCH] target/arm: Fix alignment for VLD4.32
On 9/14/22 11:50, Clément Chigot wrote:
When requested, the alignment for VLD4.32 is 8 and not 16.
See ARM documentation about VLD4 encoding:
ebytes = 1 << UInt(size);
if size == '10' then
alignment = if a == '0' then 1 else 8;
else
alignment = if a == '0' then 1 else 4*ebytes;
Signed-off-by: Clément Chigot
Reviewed-by: Richard Henderson
r~
---
target/arm/translate-neon.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/target/arm/translate-neon.c b/target/arm/translate-neon.c
index 321c17e2c7..4016339d46 100644
--- a/target/arm/translate-neon.c
+++ b/target/arm/translate-neon.c
@@ -584,7 +584,11 @@ static bool trans_VLD_all_lanes(DisasContext *s,
arg_VLD_all_lanes *a)
case 3:
return false;
case 4:
-align = pow2_align(size + 2);
+if (size == 2) {
+align = pow2_align(3);
+} else {
+align = pow2_align(size + 2);
+}
break;
default:
g_assert_not_reached();
[PULL 20/20] target/arm: Make boards pass base address to armv7m_load_kernel()
From: Peter Maydell
Currently armv7m_load_kernel() takes the size of the block of memory
where it should load the initial guest image, but assumes that it
should always load it at address 0. This happens to be true of all
our M-profile boards at the moment, but it isn't guaranteed to always
be so: M-profile CPUs can be configured (via init-svtor and
init-nsvtor, which match equivalent hardware configuration signals)
to have the initial vector table at any address, not just zero. (For
instance the Teeny board has the boot ROM at address 0x0200_.)
Add a base address argument to armv7m_load_kernel(), so that
callers now pass in both base address and size. All the current
callers pass 0, so this is not a behaviour change.
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20220823160417.3858216-3-peter.mayd...@linaro.org>
Signed-off-by: Richard Henderson
---
include/hw/arm/boot.h | 5 -
hw/arm/armv7m.c | 5 +++--
hw/arm/aspeed.c | 1 +
hw/arm/microbit.c | 2 +-
hw/arm/mps2-tz.c | 2 +-
hw/arm/mps2.c | 2 +-
hw/arm/msf2-som.c | 2 +-
hw/arm/musca.c| 3 ++-
hw/arm/netduino2.c| 2 +-
hw/arm/netduinoplus2.c| 2 +-
hw/arm/stellaris.c| 2 +-
hw/arm/stm32vldiscovery.c | 2 +-
12 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/include/hw/arm/boot.h b/include/hw/arm/boot.h
index c7ebae156e..f18cc3064f 100644
--- a/include/hw/arm/boot.h
+++ b/include/hw/arm/boot.h
@@ -25,13 +25,16 @@ typedef enum {
* armv7m_load_kernel:
* @cpu: CPU
* @kernel_filename: file to load
+ * @mem_base: base address to load image at (should be where the
+ *CPU expects to find its vector table on reset)
* @mem_size: mem_size: maximum image size to load
*
* Load the guest image for an ARMv7M system. This must be called by
* any ARMv7M board. (This is necessary to ensure that the CPU resets
* correctly on system reset, as well as for kernel loading.)
*/
-void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int
mem_size);
+void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename,
+hwaddr mem_base, int mem_size);
/* arm_boot.c */
struct arm_boot_info {
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index fa4c2c735d..50a9507c0b 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -568,7 +568,8 @@ static void armv7m_reset(void *opaque)
cpu_reset(CPU(cpu));
}
-void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename, int mem_size)
+void armv7m_load_kernel(ARMCPU *cpu, const char *kernel_filename,
+hwaddr mem_base, int mem_size)
{
ssize_t image_size;
uint64_t entry;
@@ -588,7 +589,7 @@ void armv7m_load_kernel(ARMCPU *cpu, const char
*kernel_filename, int mem_size)
, NULL, NULL,
NULL, 0, EM_ARM, 1, 0, as);
if (image_size < 0) {
-image_size = load_image_targphys_as(kernel_filename, 0,
+image_size = load_image_targphys_as(kernel_filename, mem_base,
mem_size, as);
}
if (image_size < 0) {
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index b3bbe06f8f..bc3ecdb619 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1430,6 +1430,7 @@ static void aspeed_minibmc_machine_init(MachineState
*machine)
armv7m_load_kernel(ARM_CPU(first_cpu),
machine->kernel_filename,
+ 0,
AST1030_INTERNAL_FLASH_SIZE);
}
diff --git a/hw/arm/microbit.c b/hw/arm/microbit.c
index e9494334ce..50df362088 100644
--- a/hw/arm/microbit.c
+++ b/hw/arm/microbit.c
@@ -57,7 +57,7 @@ static void microbit_init(MachineState *machine)
mr, -1);
armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
- s->nrf51.flash_size);
+ 0, s->nrf51.flash_size);
}
static void microbit_machine_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index 4017392bf5..394192b9b2 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -1197,7 +1197,7 @@ static void mps2tz_common_init(MachineState *machine)
}
armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
- boot_ram_size(mms));
+ 0, boot_ram_size(mms));
}
static void mps2_tz_idau_check(IDAUInterface *ii, uint32_t address,
diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index bb76fa6889..a86a994dba 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -450,7 +450,7 @@ static void mps2_common_init(MachineState *machine)
mmc->fpga_type == FPGA_AN511 ? 47 : 13));
armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
- 0x40);
+
Re: [PATCH v11 10/21] block/mirror.c: use of job helpers in drivers
On 8/26/22 16:20, Emanuele Giuseppe Esposito wrote:
Once job lock is used and aiocontext is removed, mirror has
to perform job operations under the same critical section,
Note: at this stage, job_{lock/unlock} and job lock guard macros
are*nop*.
Signed-off-by: Emanuele Giuseppe Esposito
Reviewed-by: Vladimir Sementsov-Ogievskiy
--
Best regards,
Vladimir
[PULL 15/20] target/arm: Rename pmu_8_n feature test functions
From: Peter Maydell
Our feature test functions that check the PMU version are named
isar_feature_{aa32,aa64,any}_pmu_8_{1,4}. This doesn't match the
current Arm ARM official feature names, which are FEAT_PMUv3p1 and
FEAT_PMUv3p4. Rename these functions to _pmuv3p1 and _pmuv3p4.
This commit was created with:
sed -i -e 's/pmu_8_/pmuv3p/g' target/arm/*.[ch]
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Message-Id: <20220822132358.3524971-8-peter.mayd...@linaro.org>
Signed-off-by: Richard Henderson
---
target/arm/cpu.h| 16
target/arm/helper.c | 18 +-
2 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index fa24ce9f96..d86e51992a 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3712,14 +3712,14 @@ static inline bool isar_feature_aa32_ats1e1(const
ARMISARegisters *id)
return FIELD_EX32(id->id_mmfr3, ID_MMFR3, PAN) >= 2;
}
-static inline bool isar_feature_aa32_pmu_8_1(const ARMISARegisters *id)
+static inline bool isar_feature_aa32_pmuv3p1(const ARMISARegisters *id)
{
/* 0xf means "non-standard IMPDEF PMU" */
return FIELD_EX32(id->id_dfr0, ID_DFR0, PERFMON) >= 4 &&
FIELD_EX32(id->id_dfr0, ID_DFR0, PERFMON) != 0xf;
}
-static inline bool isar_feature_aa32_pmu_8_4(const ARMISARegisters *id)
+static inline bool isar_feature_aa32_pmuv3p4(const ARMISARegisters *id)
{
/* 0xf means "non-standard IMPDEF PMU" */
return FIELD_EX32(id->id_dfr0, ID_DFR0, PERFMON) >= 5 &&
@@ -4038,13 +4038,13 @@ static inline bool isar_feature_aa64_sme(const
ARMISARegisters *id)
return FIELD_EX64(id->id_aa64pfr1, ID_AA64PFR1, SME) != 0;
}
-static inline bool isar_feature_aa64_pmu_8_1(const ARMISARegisters *id)
+static inline bool isar_feature_aa64_pmuv3p1(const ARMISARegisters *id)
{
return FIELD_EX64(id->id_aa64dfr0, ID_AA64DFR0, PMUVER) >= 4 &&
FIELD_EX64(id->id_aa64dfr0, ID_AA64DFR0, PMUVER) != 0xf;
}
-static inline bool isar_feature_aa64_pmu_8_4(const ARMISARegisters *id)
+static inline bool isar_feature_aa64_pmuv3p4(const ARMISARegisters *id)
{
return FIELD_EX64(id->id_aa64dfr0, ID_AA64DFR0, PMUVER) >= 5 &&
FIELD_EX64(id->id_aa64dfr0, ID_AA64DFR0, PMUVER) != 0xf;
@@ -4213,14 +4213,14 @@ static inline bool isar_feature_any_predinv(const
ARMISARegisters *id)
return isar_feature_aa64_predinv(id) || isar_feature_aa32_predinv(id);
}
-static inline bool isar_feature_any_pmu_8_1(const ARMISARegisters *id)
+static inline bool isar_feature_any_pmuv3p1(const ARMISARegisters *id)
{
-return isar_feature_aa64_pmu_8_1(id) || isar_feature_aa32_pmu_8_1(id);
+return isar_feature_aa64_pmuv3p1(id) || isar_feature_aa32_pmuv3p1(id);
}
-static inline bool isar_feature_any_pmu_8_4(const ARMISARegisters *id)
+static inline bool isar_feature_any_pmuv3p4(const ARMISARegisters *id)
{
-return isar_feature_aa64_pmu_8_4(id) || isar_feature_aa32_pmu_8_4(id);
+return isar_feature_aa64_pmuv3p4(id) || isar_feature_aa32_pmuv3p4(id);
}
static inline bool isar_feature_any_ccidx(const ARMISARegisters *id)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 11a7a57710..987ac19fe8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -879,16 +879,16 @@ static int64_t instructions_ns_per(uint64_t icount)
}
#endif
-static bool pmu_8_1_events_supported(CPUARMState *env)
+static bool pmuv3p1_events_supported(CPUARMState *env)
{
/* For events which are supported in any v8.1 PMU */
-return cpu_isar_feature(any_pmu_8_1, env_archcpu(env));
+return cpu_isar_feature(any_pmuv3p1, env_archcpu(env));
}
-static bool pmu_8_4_events_supported(CPUARMState *env)
+static bool pmuv3p4_events_supported(CPUARMState *env)
{
/* For events which are supported in any v8.1 PMU */
-return cpu_isar_feature(any_pmu_8_4, env_archcpu(env));
+return cpu_isar_feature(any_pmuv3p4, env_archcpu(env));
}
static uint64_t zero_event_get_count(CPUARMState *env)
@@ -922,17 +922,17 @@ static const pm_event pm_events[] = {
},
#endif
{ .number = 0x023, /* STALL_FRONTEND */
- .supported = pmu_8_1_events_supported,
+ .supported = pmuv3p1_events_supported,
.get_count = zero_event_get_count,
.ns_per_count = zero_event_ns_per,
},
{ .number = 0x024, /* STALL_BACKEND */
- .supported = pmu_8_1_events_supported,
+ .supported = pmuv3p1_events_supported,
.get_count = zero_event_get_count,
.ns_per_count = zero_event_ns_per,
},
{ .number = 0x03c, /* STALL */
- .supported = pmu_8_4_events_supported,
+ .supported = pmuv3p4_events_supported,
.get_count = zero_event_get_count,
.ns_per_count = zero_event_ns_per,
},
@@ -6400,7 +6400,7 @@ static void define_pmu_regs(ARMCPU *cpu)
g_free(pmevtyper_name);
g_free(pmevtyper_el0_name);
}
-if (cpu_isar_feature(aa32_pmu_8_1, cpu)) {
+if (cpu_isar_feature(aa32_pmuv3p1,
[PATCH v2] e1000e: set RX desc status with DD flag in a separate operation
Like commit 034d00d48581 ("e1000: set RX descriptor status in
a separate operation"), there is also same issue in e1000e, which
would cause lost packets or stop sending packets to VM with DPDK.
Do similar fix in e1000e.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/402
Signed-off-by: Ding Hui
---
hw/net/e1000e_core.c | 53 +++-
1 file changed, 52 insertions(+), 1 deletion(-)
---
v2: use uint8_t/uint32_t directly instead of typeof
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 208e3e0d79..a570b366b2 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1364,6 +1364,57 @@ struct NetRxPkt *pkt, const E1000E_RSSInfo *rss_info,
}
}
+static inline void
+e1000e_pci_dma_write_rx_desc(E1000ECore *core, dma_addr_t addr,
+ uint8_t *desc, dma_addr_t len)
+{
+PCIDevice *dev = core->owner;
+
+if (e1000e_rx_use_legacy_descriptor(core)) {
+struct e1000_rx_desc *d = (struct e1000_rx_desc *) desc;
+size_t offset = offsetof(struct e1000_rx_desc, status);
+uint8_t status = d->status;
+
+d->status &= ~E1000_RXD_STAT_DD;
+pci_dma_write(dev, addr, desc, len);
+
+if (status & E1000_RXD_STAT_DD) {
+d->status = status;
+pci_dma_write(dev, addr + offset, , sizeof(status));
+}
+} else {
+if (core->mac[RCTL] & E1000_RCTL_DTYP_PS) {
+union e1000_rx_desc_packet_split *d =
+(union e1000_rx_desc_packet_split *) desc;
+size_t offset = offsetof(union e1000_rx_desc_packet_split,
+wb.middle.status_error);
+uint32_t status = d->wb.middle.status_error;
+
+d->wb.middle.status_error &= ~E1000_RXD_STAT_DD;
+pci_dma_write(dev, addr, desc, len);
+
+if (status & E1000_RXD_STAT_DD) {
+d->wb.middle.status_error = status;
+pci_dma_write(dev, addr + offset, , sizeof(status));
+}
+} else {
+union e1000_rx_desc_extended *d =
+(union e1000_rx_desc_extended *) desc;
+size_t offset = offsetof(union e1000_rx_desc_extended,
+wb.upper.status_error);
+uint32_t status = d->wb.upper.status_error;
+
+d->wb.upper.status_error &= ~E1000_RXD_STAT_DD;
+pci_dma_write(dev, addr, desc, len);
+
+if (status & E1000_RXD_STAT_DD) {
+d->wb.upper.status_error = status;
+pci_dma_write(dev, addr + offset, , sizeof(status));
+}
+}
+}
+}
+
typedef struct e1000e_ba_state_st {
uint16_t written[MAX_PS_BUFFERS];
uint8_t cur_idx;
@@ -1600,7 +1651,7 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct
NetRxPkt *pkt,
e1000e_write_rx_descr(core, desc, is_last ? core->rx_pkt : NULL,
rss_info, do_ps ? ps_hdr_len : 0, );
-pci_dma_write(d, base, , core->rx_desc_len);
+e1000e_pci_dma_write_rx_desc(core, base, desc, core->rx_desc_len);
e1000e_ring_advance(core, rxi,
core->rx_desc_len / E1000_MIN_RX_DESC_LEN);
--
2.17.1
[PATCH v2 3/4] scripts/ci/setup: spice-server only on x86 aarch64
From: "Lucas Mateus Castro (alqotel)" Changed build-environment.yml to only install spice-server on x86_64 and aarch64 as this package is only available on those architectures. Signed-off-by: Lucas Mateus Castro (alqotel) --- scripts/ci/setup/build-environment.yml | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/scripts/ci/setup/build-environment.yml b/scripts/ci/setup/build-environment.yml index 7535228685..43cf8c759f 100644 --- a/scripts/ci/setup/build-environment.yml +++ b/scripts/ci/setup/build-environment.yml @@ -160,7 +160,6 @@ - python36 - rdma-core-devel - spice-glib-devel - - spice-server - systemtap-sdt-devel - tar - zlib-devel @@ -168,3 +167,14 @@ when: - ansible_facts['distribution_file_variety'] == 'RedHat' - ansible_facts['distribution_version'] == '8' + +- name: Install packages only available on x86 and aarch64 + dnf: +# Spice server not available in ppc64le +name: + - spice-server +state: present + when: +- ansible_facts['distribution_file_variety'] == 'RedHat' +- ansible_facts['distribution_version'] == '8' +- ansible_facts['architecture'] == 'aarch64' or ansible_facts['architecture'] == 'x86_64' -- 2.31.1
[PATCH] target/arm: Do alignment check when translation disabled
If translation is disabled, the default memory type is Device,
which requires alignment checking. Document, but defer, the
more general case of per-page alignment checking.
Reported-by: Idan Horowitz
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1204
Signed-off-by: Richard Henderson
---
target/arm/helper.c | 38 --
1 file changed, 36 insertions(+), 2 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index d7bc467a2a..79609443aa 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10713,6 +10713,39 @@ ARMMMUIdx arm_mmu_idx(CPUARMState *env)
return arm_mmu_idx_el(env, arm_current_el(env));
}
+/*
+ * Return true if memory alignment should be enforced.
+ */
+static bool aprofile_require_alignment(CPUARMState *env, int el, uint64_t
sctlr)
+{
+/* Check the alignment enable bit. */
+if (sctlr & SCTLR_A) {
+return true;
+}
+
+/*
+ * If translation is disabled, then the default memory type
+ * may be Device(-nGnRnE) instead of Normal, which requires that
+ * alignment be enforced.
+ *
+ * TODO: The more general case is translation enabled, with a per-page
+ * check of the memory type as assigned via MAIR_ELx and the PTE.
+ * We could arrange for a bit in MemTxAttrs to enforce alignment
+ * via forced use of the softmmu slow path. Given that such pages
+ * are intended for MMIO, where the slow path is required anyhow,
+ * this should not result in extra overhead.
+ */
+if (sctlr & SCTLR_M) {
+/* Translation enabled: memory type in PTE via MAIR_ELx. */
+return false;
+}
+if (el < 2 && (arm_hcr_el2_eff(env) & (HCR_DC | HCR_VM))) {
+/* Stage 2 translation enabled: memory type in PTE. */
+return false;
+}
+return true;
+}
+
static CPUARMTBFlags rebuild_hflags_common(CPUARMState *env, int fp_el,
ARMMMUIdx mmu_idx,
CPUARMTBFlags flags)
@@ -10777,8 +10810,9 @@ static CPUARMTBFlags rebuild_hflags_a32(CPUARMState
*env, int fp_el,
{
CPUARMTBFlags flags = {};
int el = arm_current_el(env);
+uint64_t sctlr = arm_sctlr(env, el);
-if (arm_sctlr(env, el) & SCTLR_A) {
+if (aprofile_require_alignment(env, el, sctlr)) {
DP_TBFLAG_ANY(flags, ALIGN_MEM, 1);
}
@@ -10871,7 +10905,7 @@ static CPUARMTBFlags rebuild_hflags_a64(CPUARMState
*env, int el, int fp_el,
sctlr = regime_sctlr(env, stage1);
-if (sctlr & SCTLR_A) {
+if (aprofile_require_alignment(env, el, sctlr)) {
DP_TBFLAG_ANY(flags, ALIGN_MEM, 1);
}
--
2.34.1
[PULL 14/20] target/arm: Detect overflow when calculating next PMU interrupt
From: Peter Maydell
In pmccntr_op_finish() and pmevcntr_op_finish() we calculate the next
point at which we will get an overflow and need to fire the PMU
interrupt or set the overflow flag. We do this by calculating the
number of nanoseconds to the overflow event and then adding it to
qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL). However, we don't check
whether that signed addition overflows, which can happen if the next
PMU interrupt would happen massively far in the future (250 years or
more).
Since QEMU assumes that "when the QEMU_CLOCK_VIRTUAL rolls over" is
"never", the sensible behaviour in this situation is simply to not
try to set the timer if it would be beyond that point. Detect the
overflow, and skip setting the timer in that case.
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Message-Id: <20220822132358.3524971-7-peter.mayd...@linaro.org>
Signed-off-by: Richard Henderson
---
target/arm/helper.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index b792694df0..11a7a57710 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1227,10 +1227,13 @@ static void pmccntr_op_finish(CPUARMState *env)
int64_t overflow_in = cycles_ns_per(remaining_cycles);
if (overflow_in > 0) {
-int64_t overflow_at = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-overflow_in;
-ARMCPU *cpu = env_archcpu(env);
-timer_mod_anticipate_ns(cpu->pmu_timer, overflow_at);
+int64_t overflow_at;
+
+if (!sadd64_overflow(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
+ overflow_in, _at)) {
+ARMCPU *cpu = env_archcpu(env);
+timer_mod_anticipate_ns(cpu->pmu_timer, overflow_at);
+}
}
#endif
@@ -1275,10 +1278,13 @@ static void pmevcntr_op_finish(CPUARMState *env,
uint8_t counter)
int64_t overflow_in = pm_events[event_idx].ns_per_count(delta);
if (overflow_in > 0) {
-int64_t overflow_at = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-overflow_in;
-ARMCPU *cpu = env_archcpu(env);
-timer_mod_anticipate_ns(cpu->pmu_timer, overflow_at);
+int64_t overflow_at;
+
+if (!sadd64_overflow(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
+ overflow_in, _at)) {
+ARMCPU *cpu = env_archcpu(env);
+timer_mod_anticipate_ns(cpu->pmu_timer, overflow_at);
+}
}
#endif
--
2.34.1
[PULL 19/20] target/arm: Remove useless TARGET_BIG_ENDIAN check in armv7m_load_kernel()
From: Peter Maydell
Arm system emulation targets always have TARGET_BIG_ENDIAN clear, so
there is no need to have handling in armv7m_load_kernel() for the
case when it is defined. Remove the unnecessary code.
Side notes:
* our M-profile implementation is always little-endian (that is, it
makes the IMPDEF choice that the read-only AIRCR.ENDIANNESS is 0)
* if we did want to handle big-endian ELF files here we should do it
the way that hw/arm/boot.c:arm_load_elf() does, by looking at the
ELF header to see what endianness the file itself is
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20220823160417.3858216-2-peter.mayd...@linaro.org>
Signed-off-by: Richard Henderson
---
hw/arm/armv7m.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 990861ee5e..fa4c2c735d 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -572,17 +572,10 @@ void armv7m_load_kernel(ARMCPU *cpu, const char
*kernel_filename, int mem_size)
{
ssize_t image_size;
uint64_t entry;
-int big_endian;
AddressSpace *as;
int asidx;
CPUState *cs = CPU(cpu);
-#if TARGET_BIG_ENDIAN
-big_endian = 1;
-#else
-big_endian = 0;
-#endif
-
if (arm_feature(>env, ARM_FEATURE_EL3)) {
asidx = ARMASIdx_S;
} else {
@@ -593,7 +586,7 @@ void armv7m_load_kernel(ARMCPU *cpu, const char
*kernel_filename, int mem_size)
if (kernel_filename) {
image_size = load_elf_as(kernel_filename, NULL, NULL, NULL,
, NULL, NULL,
- NULL, big_endian, EM_ARM, 1, 0, as);
+ NULL, 0, EM_ARM, 1, 0, as);
if (image_size < 0) {
image_size = load_image_targphys_as(kernel_filename, 0,
mem_size, as);
--
2.34.1
[PULL 12/20] target/arm: Ignore PMCR.D when PMCR.LC is set
From: Peter Maydell
The architecture requires that if PMCR.LC is set (for a 64-bit cycle
counter) then PMCR.D (which enables the clock divider so the counter
ticks every 64 cycles rather than every cycle) should be ignored. We
were always honouring PMCR.D; fix the bug so we correctly ignore it
in this situation.
Signed-off-by: Peter Maydell
Reviewed-by: Richard Henderson
Message-Id: <20220822132358.3524971-5-peter.mayd...@linaro.org>
Signed-off-by: Richard Henderson
---
target/arm/helper.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index a348c7407d..f1b20de16d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1172,6 +1172,17 @@ static void pmu_update_irq(CPUARMState *env)
(env->cp15.c9_pminten & env->cp15.c9_pmovsr));
}
+static bool pmccntr_clockdiv_enabled(CPUARMState *env)
+{
+/*
+ * Return true if the clock divider is enabled and the cycle counter
+ * is supposed to tick only once every 64 clock cycles. This is
+ * controlled by PMCR.D, but if PMCR.LC is set to enable the long
+ * (64-bit) cycle counter PMCR.D has no effect.
+ */
+return (env->cp15.c9_pmcr & (PMCRD | PMCRLC)) == PMCRD;
+}
+
/*
* Ensure c15_ccnt is the guest-visible count so that operations such as
* enabling/disabling the counter or filtering, modifying the count itself,
@@ -1184,8 +1195,7 @@ static void pmccntr_op_start(CPUARMState *env)
if (pmu_counter_enabled(env, 31)) {
uint64_t eff_cycles = cycles;
-if (env->cp15.c9_pmcr & PMCRD) {
-/* Increment once every 64 processor clock cycles */
+if (pmccntr_clockdiv_enabled(env)) {
eff_cycles /= 64;
}
@@ -1228,8 +1238,7 @@ static void pmccntr_op_finish(CPUARMState *env)
#endif
uint64_t prev_cycles = env->cp15.c15_ccnt_delta;
-if (env->cp15.c9_pmcr & PMCRD) {
-/* Increment once every 64 processor clock cycles */
+if (pmccntr_clockdiv_enabled(env)) {
prev_cycles /= 64;
}
env->cp15.c15_ccnt_delta = prev_cycles - env->cp15.c15_ccnt;
--
2.34.1
[PULL 18/20] target/arm: Report FEAT_PMUv3p5 for TCG '-cpu max'
From: Peter Maydell Update the ID registers for TCG's '-cpu max' to report a FEAT_PMUv3p5 compliant PMU. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-Id: <20220822132358.3524971-11-peter.mayd...@linaro.org> Signed-off-by: Richard Henderson --- docs/system/arm/emulation.rst | 1 + target/arm/cpu64.c| 2 +- target/arm/cpu_tcg.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst index 811358fd0a..be7bbffe59 100644 --- a/docs/system/arm/emulation.rst +++ b/docs/system/arm/emulation.rst @@ -53,6 +53,7 @@ the following architecture extensions: - FEAT_PMULL (PMULL, PMULL2 instructions) - FEAT_PMUv3p1 (PMU Extensions v3.1) - FEAT_PMUv3p4 (PMU Extensions v3.4) +- FEAT_PMUv3p5 (PMU Extensions v3.5) - FEAT_RAS (Reliability, availability, and serviceability) - FEAT_RASv1p1 (RAS Extension v1.1) - FEAT_RDM (Advanced SIMD rounding double multiply accumulate instructions) diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 3ac5e197a7..e6314e86d2 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -1152,7 +1152,7 @@ static void aarch64_max_initfn(Object *obj) t = cpu->isar.id_aa64dfr0; t = FIELD_DP64(t, ID_AA64DFR0, DEBUGVER, 9); /* FEAT_Debugv8p4 */ -t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 5);/* FEAT_PMUv3p4 */ +t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 6);/* FEAT_PMUv3p5 */ cpu->isar.id_aa64dfr0 = t; t = cpu->isar.id_aa64smfr0; diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c index b714c61d94..98b5ba2160 100644 --- a/target/arm/cpu_tcg.c +++ b/target/arm/cpu_tcg.c @@ -85,7 +85,7 @@ void aa32_max_features(ARMCPU *cpu) t = cpu->isar.id_dfr0; t = FIELD_DP32(t, ID_DFR0, COPDBG, 9);/* FEAT_Debugv8p4 */ t = FIELD_DP32(t, ID_DFR0, COPSDBG, 9); /* FEAT_Debugv8p4 */ -t = FIELD_DP32(t, ID_DFR0, PERFMON, 5); /* FEAT_PMUv3p4 */ +t = FIELD_DP32(t, ID_DFR0, PERFMON, 6); /* FEAT_PMUv3p5 */ cpu->isar.id_dfr0 = t; } -- 2.34.1
Re: [PATCH v3 00/20] ppc4xx_sdram QOMify and clean ups
On Wed, 14 Sep 2022, Cédric Le Goater wrote: On 9/13/22 21:52, BALATON Zoltan wrote: This is the end of the QOMify series started by Cédric. This series handles the SDRAM controller models to clean them up, QOMify and unify them and at least partially clean up the mess that has accumulated around these in the past. This includes the not yet merged patches from the last series and new ones that change the DDR2 version used by sam460ex. I made comments on the first ~10 patches. Let's try to agree on these first. We will see the remaining ones in a second patchset. Patch 10 does not make much sense without 11 and the final unificatoin of the two controllers is the real goal here so please try to review further patches too. Regards, BALATON Zoltan Thanks, C. v3: Fix patches that got squashed during rebase v2: address some review comments and try to avoid compile problem with gcc 12.2 (untested) BALATON Zoltan (20): ppc440_bamboo: Remove unnecessary memsets ppc4xx: Introduce Ppc4xxSdramBank struct ppc4xx_sdram: Get rid of the init RAM hack ppc4xx: Use Ppc4xxSdramBank in ppc4xx_sdram_banks() ppc440_bamboo: Add missing 4 MiB valid memory size ppc4xx_sdram: Move size check to ppc4xx_sdram_init() ppc4xx_sdram: QOM'ify ppc4xx_sdram: Drop extra zeros for readability ppc440_sdram: Split off map/unmap of sdram banks for later reuse ppc440_sdram: Implement enable bit in the DDR2 SDRAM ppc440_sdram: Get rid of the init RAM hack ppc440_sdram: Rename local variable for readibility ppc4xx_sdram: Rename functions to prevent name clashes ppc440_sdram: Move RAM size check to ppc440_sdram_init ppc440_sdram: QOM'ify ppc4xx_sdram: Move ppc4xx DDR and DDR2 SDRAM controller models together ppc4xx_sdram: Use hwaddr for memory bank size ppc4xx_sdram: Rename local state variable for brevity ppc4xx_sdram: Generalise bank setup ppc4xx_sdram: Convert DDR SDRAM controller to new bank handling hw/ppc/meson.build | 3 +- hw/ppc/ppc405.h | 8 +- hw/ppc/ppc405_boards.c | 22 +- hw/ppc/ppc405_uc.c | 33 +- hw/ppc/ppc440.h | 4 - hw/ppc/ppc440_bamboo.c | 29 +- hw/ppc/ppc440_uc.c | 267 +-- hw/ppc/ppc4xx_devs.c| 413 --- hw/ppc/ppc4xx_sdram.c | 723 hw/ppc/sam460ex.c | 48 +-- hw/ppc/trace-events | 1 + include/hw/ppc/ppc4xx.h | 66 +++- 12 files changed, 847 insertions(+), 770 deletions(-) create mode 100644 hw/ppc/ppc4xx_sdram.c
[PULL 08/20] target/arm: Add missing space in comment
From: Peter Maydell Fix a missing space before a comment terminator. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-Id: <20220819110052.2942289-7-peter.mayd...@linaro.org> Signed-off-by: Richard Henderson --- target/arm/cpu_tcg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c index f63f8cdd95..b714c61d94 100644 --- a/target/arm/cpu_tcg.c +++ b/target/arm/cpu_tcg.c @@ -64,7 +64,7 @@ void aa32_max_features(ARMCPU *cpu) t = FIELD_DP32(t, ID_MMFR4, HPDS, 1); /* FEAT_AA32HPD */ t = FIELD_DP32(t, ID_MMFR4, AC2, 1); /* ACTLR2, HACTLR2 */ t = FIELD_DP32(t, ID_MMFR4, CNP, 1); /* FEAT_TTCNP */ -t = FIELD_DP32(t, ID_MMFR4, XNX, 1); /* FEAT_XNX*/ +t = FIELD_DP32(t, ID_MMFR4, XNX, 1); /* FEAT_XNX */ cpu->isar.id_mmfr4 = t; t = cpu->isar.id_mmfr5; -- 2.34.1
