Re: Call for GSoC and Outreachy project ideas for summer 2023

2023-02-05 Thread Eugenio Perez Martin
On Fri, Jan 27, 2023 at 4:18 PM Stefan Hajnoczi  wrote:
>
> Dear QEMU, KVM, and rust-vmm communities,
> QEMU will apply for Google Summer of Code 2023
> (https://summerofcode.withgoogle.com/) and has been accepted into
> Outreachy May 2023 (https://www.outreachy.org/). You can now
> submit internship project ideas for QEMU, KVM, and rust-vmm!
>
> Please reply to this email by February 6th with your project ideas.
>
> If you have experience contributing to QEMU, KVM, or rust-vmm you can
> be a mentor. Mentors support interns as they work on their project. It's a
> great way to give back and you get to work with people who are just
> starting out in open source.
>
> Good project ideas are suitable for remote work by a competent
> programmer who is not yet familiar with the codebase. In
> addition, they are:
> - Well-defined - the scope is clear
> - Self-contained - there are few dependencies
> - Uncontroversial - they are acceptable to the community
> - Incremental - they produce deliverables along the way
>
> Feel free to post ideas even if you are unable to mentor the project.
> It doesn't hurt to share the idea!
>
> I will review project ideas and keep you up-to-date on QEMU's
> acceptance into GSoC.
>
> Internship program details:
> - Paid, remote work open source internships
> - GSoC projects are 175 or 350 hours, Outreachy projects are 30
> hrs/week for 12 weeks
> - Mentored by volunteers from QEMU, KVM, and rust-vmm
> - Mentors typically spend at least 5 hours per week during the coding period
>
> For more background on QEMU internships, check out this video:
> https://www.youtube.com/watch?v=xNVCX7YMUL8
>
> Please let me know if you have any questions!
>
> Stefan
>

Appending the different ideas here.

VIRTIO_F_IN_ORDER feature support for virtio devices
===
This was already a project the last year, and it produced a few series
upstream but was never merged. The previous series are totally useful
to start with, so it's not starting from scratch with them [1]:

Summary
---
Implement VIRTIO_F_IN_ORDER in QEMU and Linux (vhost and virtio drivers)

The VIRTIO specification defines a feature bit (VIRTIO_F_IN_ORDER)
that devices and drivers can negotiate when the device uses
descriptors in the same order in which they were made available by the
driver.

This feature can simplify device and driver implementations and
increase performance. For example, when VIRTIO_F_IN_ORDER is
negotiated, it may be easier to create a batch of buffers and reduce
DMA transactions when the device uses a batch of buffers.

Currently the devices and drivers available in Linux and QEMU do not
support this feature. An implementation is available in DPDK for the
virtio-net driver.

Goals
---
Implement VIRTIO_F_IN_ORDER for a single device/driver in QEMU and
Linux (virtio-net or virtio-serial are good starting points).
Generalize your approach to the common virtio core code for split and
packed virtqueue layouts.
If time allows, support for the packed virtqueue layout can be added
to Linux vhost, QEMU's libvhost-user, and/or QEMU's virtio qtest code.

Shadow Virtqueue missing virtio features
===

Summary
---
Some VirtIO devices like virtio-net have a control virtqueue (CVQ)
that allows them to dynamically change a number of parameters like MAC
or number of active queues. Changes to passthrough devices using vDPA
using CVQ are inherently hard to track if CVQ is handled as
passthrough data queues, because qemu is not aware of that
communication for performance reasons. In this situation, qemu is not
able to migrate these devices, as it is not able to tell the actual
state of the device.

Shadow Virtqueue (SVQ) allows qemu to offer an emulated queue to the
device, effectively forwarding the descriptors of that communication,
tracking the device internal state, and being able to migrate it to a
new destination qemu.

To restore that state in the destination, SVQ is able to send these
messages as regular CVQ commands. The code to understand and parse
virtio-net CVQ commands is already in qemu as part of its emulated
device, but the code to send the some of the new state is not, and
some features are missing. There is already code to restore basic
commands like mac or multiqueue, and it is easy to use it as a
template.

Goals
---
To implement missing virtio-net commands sending:
* VIRTIO_NET_CTRL_RX family, to control receive mode.
* VIRTIO_NET_CTRL_GUEST_OFFLOADS
* VIRTIO_NET_CTRL_VLAN family
* VIRTIO_NET_CTRL_MQ_HASH config
* VIRTIO_NET_CTRL_MQ_RSS config

Shadow Virtqueue performance optimization
===
Summary
---
To perform a virtual machine live migration with an external device to
qemu, qemu needs a way to know which memory the device modifies so it
is able to resend it. Otherwise the guest would resume with invalid /
outdated memory in the destination.

This is especially hard with passthrough hardware devices, as
transports like PCI imposes a few security and performance challenges.
As a method to overcome this for virtio devices, q

Re: [PATCH 13/19] hw/m68k: Set QDev properties using QDev API

2023-02-05 Thread Thomas Huth
Am Fri,  3 Feb 2023 19:09:08 +0100
schrieb Philippe Mathieu-Daudé :

> No need to use the low-level QOM API when an object
> inherits from QDev. Directly use the QDev API to set
> its properties.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/m68k/next-cube.c | 2 +-
>  hw/m68k/q800.c  | 7 +++
>  2 files changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Thomas Huth 



Re: [PATCH 0/3] Misc sm501 clean ups

2023-02-05 Thread Daniel Henrique Barboza




On 1/30/23 06:43, Daniel Henrique Barboza wrote:



On 1/28/23 23:43, BALATON Zoltan wrote:

On Mon, 23 Jan 2023, Philippe Mathieu-Daudé wrote:

On 21/1/23 21:35, BALATON Zoltan wrote:

Some small trivial clean ups I've found while looking at this file.

BALATON Zoltan (3):
   hw/display/sm501: Remove parenthesis around consant macro definitions
   hw/display/sm501: Remove unneeded casts from void pointer
   hw/display/sm501: Code style fix

  hw/display/sm501.c | 419 +++--
  1 file changed, 210 insertions(+), 209 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 


Ping? Who will merge this series? Should Daniel take it via PPC or Gerd for 
display? I only care that it gets in one way or another and not lost between 
maintainers.


I'm planning a PR at the end of this week. I can take these in.



Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,


Daniel




Thanks,

Daniel



Regards,
BALATON Zoltan




[RFC PATCH 01/16] hw/arm/smmuv3: Add missing fields for IDR0

2023-02-05 Thread Mostafa Saleh
In preparation for adding stage-2 support.
Add IDR0 fields related to stage-2.

VMID16: 16-bit VMID supported.
S2P: Stage-2 translation supported.

They are described in 6.3.1 SMMU_IDR0.

No functional change intended.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmuv3-internal.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index bce161870f..170e88c24a 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -34,10 +34,12 @@ typedef enum SMMUTranslationStatus {
 /* MMIO Registers */
 
 REG32(IDR0,0x0)
+FIELD(IDR0, S2P, 0 , 1)
 FIELD(IDR0, S1P, 1 , 1)
 FIELD(IDR0, TTF, 2 , 2)
 FIELD(IDR0, COHACC,  4 , 1)
 FIELD(IDR0, ASID16,  12, 1)
+FIELD(IDR0, VMID16,  18, 1)
 FIELD(IDR0, TTENDIAN,21, 2)
 FIELD(IDR0, STALL_MODEL, 24, 2)
 FIELD(IDR0, TERM_MODEL,  26, 1)
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 08/16] hw/arm/smmuv3: Support S2AFFD

2023-02-05 Thread Mostafa Saleh
Parse S2AFFD from STE and use it in stage-2 translation.

This is described in the SMMUv3 manual "5.2. Stream Table Entry" in
"[181] S2AFFD".

HTTU is not supported, SW is expected to maintain the Access flag.

This flag determines the behavior on access of a stage-2 page whose
descriptor has AF == 0:
- 0b0: An Access flag fault occurs (stall not supported).
- 0b1: An Access flag fault never occurs.

An Access fault takes priority over a Permission fault.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmu-common.c | 10 ++
 hw/arm/smmu-internal.h   |  2 ++
 hw/arm/smmuv3-internal.h |  1 +
 hw/arm/smmuv3.c  |  2 ++
 4 files changed, 15 insertions(+)

diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index df0d1dc024..541c427684 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -434,6 +434,16 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
  pte_addr, pte, iova, gpa,
  block_size >> 20);
 }
+
+/*
+ * If S2AFFD and PTE.AF are 0 => fault. (5.2. Stream Table Entry)
+ * An Access fault takes priority over a Permission fault.
+ */
+if (!PTE_AF(pte) && !cfg->s2cfg.affd) {
+info->type = SMMU_PTW_ERR_ACCESS;
+goto error;
+}
+
 ap = PTE_AP(pte);
 if (is_permission_fault_s2(ap, perm)) {
 info->type = SMMU_PTW_ERR_PERMISSION;
diff --git a/hw/arm/smmu-internal.h b/hw/arm/smmu-internal.h
index b02c05319f..7d3f76ce14 100644
--- a/hw/arm/smmu-internal.h
+++ b/hw/arm/smmu-internal.h
@@ -66,6 +66,8 @@
 #define PTE_APTABLE(pte) \
 (extract64(pte, 61, 2))
 
+#define PTE_AF(pte) \
+(extract64(pte, 10, 1))
 /*
  * TODO: At the moment all transactions are considered as privileged (EL1)
  * as IOMMU translation callback does not pass user/priv attributes.
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index ec64fb43a0..3ccb9d118e 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -524,6 +524,7 @@ typedef struct CD {
 #define STE_S2TG(x)extract32((x)->word[5], 14, 2)
 #define STE_S2PS(x)extract32((x)->word[5], 16, 3)
 #define STE_S2AA64(x)  extract32((x)->word[5], 19, 1)
+#define STE_S2AFFD(x)  extract32((x)->word[5], 21, 1)
 #define STE_S2HD(x)extract32((x)->word[5], 24, 1)
 #define STE_S2HA(x)extract32((x)->word[5], 25, 1)
 #define STE_S2S(x) extract32((x)->word[5], 26, 1)
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index c49b341287..7884401475 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -436,6 +436,8 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
 goto bad_ste;
 }
 
+cfg->s2cfg.affd = STE_S2AFFD(ste);
+
 /* This is still here as stage 2 has not been fully enabled yet. */
 qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
 goto bad_ste;
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 07/16] hw/arm/smmuv3: Check validity of stage-2 page table

2023-02-05 Thread Mostafa Saleh
Check if with the configured start level, ia_bits and granularity we
can have a valid page table as described in ARM ARM D8.2 Translation
process.
The idea is to see for the highest possible number of IPA bits, how
many concatenated tables we would need, if it is more than 16, then
this is not possible.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmuv3.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 6633fe40fa..c49b341287 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -341,6 +341,28 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t 
ssid,
 return 0;
 }
 
+/*
+ * Return true if s2 page table config is valid.
+ * This checks with the configured start level, ia_bits and granularity we can
+ * have a valid page table as described in ARM ARM D8.2 Translation process.
+ * The idea here is to see for the highest possible number of IPA bits, how
+ * many concatenated tables we would need, if it is more than 16, then this is
+ * not possible.
+ */
+static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t t0sz, uint8_t gran)
+{
+int level = get_start_level(sl0, gran);
+uint64_t ia_bits = 64 - t0sz;
+uint64_t mx = (1ULL << ia_bits) - 1;
+int nr_concat = pgd_idx(level, gran, mx) + 1;
+
+if (nr_concat > SMMU_MAX_S2_CONCAT) {
+return false;
+}
+
+return true;
+}
+
 /* Returns < 0 in case of invalid STE, 0 otherwise */
 static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
   STE *ste, SMMUEventInfo *event)
@@ -407,6 +429,13 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
 goto bad_ste;
 }
 
+if (!s2_pgtable_config_valid(cfg->s2cfg.sl0, cfg->s2cfg.tsz,
+ cfg->s2cfg.granule_sz)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+  "SMMUv3 STE stage 2 config not valid!\n");
+goto bad_ste;
+}
+
 /* This is still here as stage 2 has not been fully enabled yet. */
 qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
 goto bad_ste;
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 00/16] Add stage-2 translation for SMMUv3

2023-02-05 Thread Mostafa Saleh
This patch series adds stage-2 translation support for SMMUv3. It is
controlled by a new system property “arm-smmuv3.stage”.
- When set to “1”: Stage-1 only would be advertised and supported (default
behavior)
- When set to “2”: Stage-2 only would be advertised and supported.
- Value “all” is reserved for nesting support. However it is not
implemented in this patch series (more about this in the end)

Features implemented in stage-2 are mostly synonymous with stage-1
- VMID16.
- Only AArch64 translation tables are supported.
- 48 bits of IPA.
- Stall is not supported.
- HTTU is not supported, SW is expected to maintain the Access flag.

To make it easy to support nesting, a new structure(SMMUS2Cfg) is
embedded within SMMUTransCfg, to hold stage-2 configuration.

TLBs were updated to support VMID, where when stage-2 is used ASID are
set to -1 and ignored and when stage-1 is used VMID is set to -1 and
ignored.
As only one stage is supported at a time at the moment, TLB will
represent IPA=>PA translation with proper attributes(granularity and
t0sz) parsed from STEs for stage-2, and will represent VA=>PA
translation with proper attributes parsed from the CDs for stage-1.

New commands where added that are used with stage-2
- CMD_TLBI_S12_VMALL: Invalidate all translations for a VMID.
- CMD_TLBI_S2_IPA: Invalidate stage-2 by VMID and IPA

SMMUv3State.features has 2 new flags (SMMU_FEATURE_STAGE1 and
SMMU_FEATURE_STAGE2): to indicate stage-1 and stage-2 support in HW.

This patch series + GBPA patch
https://lore.kernel.org/qemu-devel/20230126141120.448641-1-smost...@google.com/
Can be used to run Linux pKVM SMMUv3 patches (currently on the list)
which controls stage-2 (from EL2) while providing a paravirtualized
interface the host(EL1)
https://lore.kernel.org/kvmarm/20230201125328.2186498-1-jean-phili...@linaro.org/

Looking forward, nesting is the next feature to go for, here are some
thoughts about this:

- TLB would require big changes for this, we can go either for a combined
implementation or per stage one. This would affect returns from PTW and
invalidation commands.

- Stage-1 data structures should be translated by stage-2 if enabled (as
context descriptors and ttb0/ttb1)

- Translated addresses from stage-1 should be translated by stage-2 if
enabled.

- Record faults should be separated between stage-1 (CD_R) and stage-2
(S2R).

- Some existing commands(as CMD_TLBI_S2_IPA, CMD_TLBI_NH_ASID …) would be
modified and some of those would be based on the design of the TLBs.

- Currently, VMID is ignored when stage-1 is used as it can’t be used with
stage-2. However when nesting is advertised VMID shouldn’t be ignored
even if stage-2 is bypassed.

Mostafa Saleh (16):
  hw/arm/smmuv3: Add missing fields for IDR0
  hw/arm/smmuv3: Update translation config to hold stage-2
  hw/arm/smmuv3: Rename smmu_ptw_64
  hw/arm/smmuv3: Add a system property to choose translation stage
  hw/arm/smmuv3: Add page table walk for stage-2
  hw/arm/smmuv3: Parse STE config for stage-2
  hw/arm/smmuv3: Check validity of stage-2 page table
  hw/arm/smmuv3: Support S2AFFD
  hw/arm/smmuv3: Don't touch CD if stage-1 is not supported.
  hw/arm/smmuv3: Make TLB lookup work for stage-2
  hw/arm/smmuv3: Read VMID from STE
  hw/arm/smmuv3: Add VMID to tlb tagging
  hw/arm/smmuv3: Add CMDs related to stage 2
  hw/arm/smmuv3: Add stage-2 support in iova notifier
  hw/arm/smmuv3: Add fault configuration for stage-2
  hw/arm/smmuv3: Enable stage-2 support

 hw/arm/smmu-common.c | 168 +---
 hw/arm/smmu-internal.h   |  41 ++
 hw/arm/smmuv3-internal.h |  10 ++
 hw/arm/smmuv3.c  | 247 ++-
 hw/arm/trace-events  |   4 +-
 include/hw/arm/smmu-common.h |  19 ++-
 include/hw/arm/smmuv3.h  |   1 +
 7 files changed, 433 insertions(+), 57 deletions(-)

-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 04/16] hw/arm/smmuv3: Add a system property to choose translation stage

2023-02-05 Thread Mostafa Saleh
Add a new system property for smmuv3 to choose what translation
stages to advertise.

The property added arm-smmuv3.stage can have 3 values:
- "1": Stage-1 is only advertised.
- "2": Stage-2 is only advertised.
- "all": Stage-1 + Stage-2 are supported, which is not implemented in
this patch series.

If not passed or an unsupported value is passed, it will default to
stage-1.

The property is not used in this patch as stage-2 has not been
enabled yet.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmuv3-internal.h |  5 +
 hw/arm/smmuv3.c  | 28 +++-
 include/hw/arm/smmuv3.h  |  1 +
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index 170e88c24a..ec64fb43a0 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -329,6 +329,11 @@ enum { /* Command completion notification */
 })
 
 #define SMMU_FEATURE_2LVL_STE (1 << 0)
+#define SMMU_FEATURE_STAGE1   (1 << 1)
+#define SMMU_FEATURE_STAGE2   (1 << 2)
+
+#define STAGE1_SUPPORTED(f)   (f & SMMU_FEATURE_STAGE1)
+#define STAGE2_SUPPORTED(f)   (f & SMMU_FEATURE_STAGE2)
 
 /* Events */
 
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 955b89c8d5..54dd8e5ec1 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -21,6 +21,7 @@
 #include "hw/irq.h"
 #include "hw/sysbus.h"
 #include "migration/vmstate.h"
+#include "hw/qdev-properties.h"
 #include "hw/qdev-core.h"
 #include "hw/pci/pci.h"
 #include "cpu.h"
@@ -238,6 +239,19 @@ void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo 
*info)
 
 static void smmuv3_init_regs(SMMUv3State *s)
 {
+/*
+ * Based on system property, the stages supported in smmu will be 
advertised.
+ * At the moment "all" is not supported.
+ * Default stage is 1.
+ */
+s->features = SMMU_FEATURE_STAGE1;
+if (s->stage && !strcmp("2", s->stage)) {
+s->features = SMMU_FEATURE_STAGE2;
+} else if (s->stage && !strcmp("all", s->stage)) {
+qemu_log_mask(LOG_UNIMP,
+  "SMMUv3 S1 and S2 nesting not supported, defaulting 
to S1\n");
+}
+
 /**
  * IDR0: stage1 only, AArch64 only, coherent access, 16b ASID,
  *   multi-level stream table
@@ -276,7 +290,6 @@ static void smmuv3_init_regs(SMMUv3State *s)
 s->eventq.cons = 0;
 s->eventq.entry_size = sizeof(struct Evt);
 
-s->features = 0;
 s->sid_split = 0;
 s->aidr = 0x1;
 s->cr[0] = 0;
@@ -1514,6 +1527,18 @@ static const VMStateDescription vmstate_smmuv3 = {
 },
 };
 
+static Property smmuv3_properties[] = {
+/*
+ * Stages of translation advertised.
+ * "1": Stage 1
+ * "2": Stage 2
+ * "all": Stage 1 + Stage 2
+ * Defaults to stage 1
+ */
+DEFINE_PROP_STRING("stage", SMMUv3State, stage),
+DEFINE_PROP_END_OF_LIST()
+};
+
 static void smmuv3_instance_init(Object *obj)
 {
 /* Nothing much to do here as of now */
@@ -1530,6 +1555,7 @@ static void smmuv3_class_init(ObjectClass *klass, void 
*data)
&c->parent_phases);
 c->parent_realize = dc->realize;
 dc->realize = smmu_realize;
+device_class_set_props(dc, smmuv3_properties);
 }
 
 static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu,
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index f1921fdf9e..9b9e1c7baf 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -62,6 +62,7 @@ struct SMMUv3State {
 
 qemu_irq irq[4];
 QemuMutex mutex;
+char *stage;
 };
 
 typedef enum {
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 09/16] hw/arm/smmuv3: Don't touch CD if stage-1 is not supported.

2023-02-05 Thread Mostafa Saleh
If stage-1 is not supported, SW will not configure it, so don't try to
access it as it might have faulty addresses.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmuv3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 7884401475..c18460a4ff 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -653,7 +653,7 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, 
SMMUTransCfg *cfg,
 return ret;
 }
 
-if (cfg->aborted || cfg->bypassed) {
+if (cfg->aborted || cfg->bypassed || !STAGE1_SUPPORTED(s->features)) {
 return 0;
 }
 
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 03/16] hw/arm/smmuv3: Rename smmu_ptw_64

2023-02-05 Thread Mostafa Saleh
In preparation for adding stage-2 support. Rename smmu_ptw_64 to
smmu_ptw_64_s1.

No functional change intended.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmu-common.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 54186f31cb..4fcbffa2f1 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -264,7 +264,7 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t 
iova)
 }
 
 /**
- * smmu_ptw_64 - VMSAv8-64 Walk of the page tables for a given IOVA
+ * smmu_ptw_64_s1 - VMSAv8-64 Walk of the page tables for a given IOVA
  * @cfg: translation config
  * @iova: iova to translate
  * @perm: access type
@@ -276,9 +276,9 @@ SMMUTransTableInfo *select_tt(SMMUTransCfg *cfg, dma_addr_t 
iova)
  * Upon success, @tlbe is filled with translated_addr and entry
  * permission rights.
  */
-static int smmu_ptw_64(SMMUTransCfg *cfg,
-   dma_addr_t iova, IOMMUAccessFlags perm,
-   SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
+static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
+  dma_addr_t iova, IOMMUAccessFlags perm,
+  SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
 {
 dma_addr_t baseaddr, indexmask;
 int stage = cfg->stage;
@@ -384,7 +384,7 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, 
IOMMUAccessFlags perm,
 g_assert_not_reached();
 }
 
-return smmu_ptw_64(cfg, iova, perm, tlbe, info);
+return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
 }
 
 /**
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 13/16] hw/arm/smmuv3: Add CMDs related to stage 2

2023-02-05 Thread Mostafa Saleh
CMD_TLBI_S2_IPA: As S1+S2 is not enabled, for now this can be the
same as CMD_TLBI_NH_VAA.

CMD_TLBI_S12_VMALL: Added new function to invalidate TLB by VMID.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmu-common.c | 16 
 hw/arm/smmuv3.c  | 25 +++--
 hw/arm/trace-events  |  2 ++
 include/hw/arm/smmu-common.h |  1 +
 4 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 028a60949a..28089d94a6 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -133,6 +133,16 @@ static gboolean smmu_hash_remove_by_asid(gpointer key, 
gpointer value,
 
 return SMMU_IOTLB_ASID(*iotlb_key) == asid;
 }
+
+static gboolean smmu_hash_remove_by_vmid(gpointer key, gpointer value,
+ gpointer user_data)
+{
+uint16_t vmid = *(uint16_t *)user_data;
+SMMUIOTLBKey *iotlb_key = (SMMUIOTLBKey *)key;
+
+return SMMU_IOTLB_VMID(*iotlb_key) == vmid;
+}
+
 static gboolean smmu_hash_remove_by_asid_vmid_iova(gpointer key, gpointer 
value,
   gpointer user_data)
 {
@@ -185,6 +195,12 @@ void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid)
 g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_asid, &asid);
 }
 
+inline void smmu_iotlb_inv_vmid(SMMUState *s, uint16_t vmid)
+{
+trace_smmu_iotlb_inv_vmid(vmid);
+g_hash_table_foreach_remove(s->iotlb, smmu_hash_remove_by_vmid, &vmid);
+}
+
 /* VMSAv8-64 Translation */
 
 /**
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 8b070f6bb5..2b563a5b1b 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1174,14 +1174,35 @@ static int smmuv3_cmdq_consume(SMMUv3State *s)
 case SMMU_CMD_TLBI_NH_VA:
 smmuv3_s1_range_inval(bs, &cmd);
 break;
+case SMMU_CMD_TLBI_S12_VMALL:
+uint16_t vmid = CMD_VMID(&cmd);
+
+if (!STAGE2_SUPPORTED(s->features)) {
+cmd_error = SMMU_CERROR_ILL;
+break;
+}
+
+trace_smmuv3_cmdq_tlbi_s12_vmid(vmid);
+smmu_inv_notifiers_all(&s->smmu_state);
+smmu_iotlb_inv_vmid(bs, vmid);
+break;
+case SMMU_CMD_TLBI_S2_IPA:
+if (!STAGE2_SUPPORTED(s->features)) {
+cmd_error = SMMU_CERROR_ILL;
+break;
+}
+/*
+ * As currently only either s1 or s2 are supported
+ * we can reuse same function for s2.
+ */
+smmuv3_s1_range_inval(bs, &cmd);
+break;
 case SMMU_CMD_TLBI_EL3_ALL:
 case SMMU_CMD_TLBI_EL3_VA:
 case SMMU_CMD_TLBI_EL2_ALL:
 case SMMU_CMD_TLBI_EL2_ASID:
 case SMMU_CMD_TLBI_EL2_VA:
 case SMMU_CMD_TLBI_EL2_VAA:
-case SMMU_CMD_TLBI_S12_VMALL:
-case SMMU_CMD_TLBI_S2_IPA:
 case SMMU_CMD_ATC_INV:
 case SMMU_CMD_PRI_RESP:
 case SMMU_CMD_RESUME:
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 2dee296c8f..61e2ffade5 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -12,6 +12,7 @@ smmu_ptw_block_pte(int stage, int level, uint64_t baseaddr, 
uint64_t pteaddr, ui
 smmu_get_pte(uint64_t baseaddr, int index, uint64_t pteaddr, uint64_t pte) 
"baseaddr=0x%"PRIx64" index=0x%x, pteaddr=0x%"PRIx64", pte=0x%"PRIx64
 smmu_iotlb_inv_all(void) "IOTLB invalidate all"
 smmu_iotlb_inv_asid(uint16_t asid) "IOTLB invalidate asid=%d"
+smmu_iotlb_inv_vmid(uint16_t vmid) "IOTLB invalidate vmid=%d"
 smmu_iotlb_inv_iova(uint16_t asid, uint64_t addr) "IOTLB invalidate asid=%d 
addr=0x%"PRIx64
 smmu_inv_notifiers_mr(const char *name) "iommu mr=%s"
 smmu_iotlb_lookup_hit(uint16_t asid, uint64_t addr, uint32_t hit, uint32_t 
miss, uint32_t p) "IOTLB cache HIT asid=%d addr=0x%"PRIx64" hit=%d miss=%d hit 
rate=%d"
@@ -48,6 +49,7 @@ smmuv3_config_cache_miss(uint32_t sid, uint32_t hits, 
uint32_t misses, uint32_t
 smmuv3_s1_range_inval(int vmid, int asid, uint64_t addr, uint8_t tg, uint64_t 
num_pages, uint8_t ttl, bool leaf) "vmid=%d asid=%d addr=0x%"PRIx64" tg=%d 
num_pages=0x%"PRIx64" ttl=%d leaf=%d"
 smmuv3_cmdq_tlbi_nh(void) ""
 smmuv3_cmdq_tlbi_nh_asid(uint16_t asid) "asid=%d"
+smmuv3_cmdq_tlbi_s12_vmid(uint16_t vmid) "vmid=%d"
 smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid=0x%x"
 smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu 
mr=%s"
 smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu 
mr=%s"
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 5cca1c17f5..46ba1f6329 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -181,6 +181,7 @@ SMMUIOTLBKey smmu_get_iotlb_key(uint16_t asid, uint16_t 
vmid, uint64_t iova,
 uint8_t tg, uint8_t level);
 void smmu_iotlb_inv_all(SMMUState *s);
 void smmu_iotlb_inv_asid(SMMUState *s, uint16_t asid);

[RFC PATCH 06/16] hw/arm/smmuv3: Parse STE config for stage-2

2023-02-05 Thread Mostafa Saleh
Parse stage-2 configuration and populate it in SMMUTransCfg.
Configs in this patch (s2g, ttb, tsz, sl0).
Checking validity of values added when possible.

MAX IPA supported is 48 bits and only AA64 tables are supported.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmuv3.c  | 43 +++-
 include/hw/arm/smmu-common.h |  1 +
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 54dd8e5ec1..6633fe40fa 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -366,7 +366,48 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
 return 0;
 }
 
-if (STE_CFG_S2_ENABLED(config)) {
+if (STAGE2_SUPPORTED(s->features) && STE_CFG_S2_ENABLED(config)) {
+cfg->stage = 2;
+
+if (STE_S2AA64(ste) == 0x0) {
+qemu_log_mask(LOG_UNIMP,
+  "SMMUv3 AArch32 tables not supported\n");
+goto bad_ste;
+}
+
+switch (STE_S2TG(ste)) {
+case 0x0: /* 4KB */
+cfg->s2cfg.granule_sz = 12;
+break;
+case 0x1: /* 64KB */
+cfg->s2cfg.granule_sz = 16;
+break;
+case 0x2: /* 16KB */
+cfg->s2cfg.granule_sz = 14;
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+  "SMMUv3 bad STE S2TG: %x\n", STE_S2TG(ste));
+goto bad_ste;
+}
+
+cfg->s2cfg.vttb = STE_S2TTB(ste);
+cfg->s2cfg.tsz = STE_S2T0SZ(ste);
+
+if ((64 - cfg->s2cfg.tsz) > SMMU_MAX_IPA_BITS) {
+qemu_log_mask(LOG_UNIMP, "SMMUv3 IPA too big! TS0Z = %x\n",
+  cfg->s2cfg.tsz);
+goto bad_ste;
+}
+
+cfg->s2cfg.sl0 = STE_S2SL0(ste);
+if (cfg->s2cfg.sl0 == 0x3) {
+qemu_log_mask(LOG_UNIMP,
+  "SMMUv3 STE->SL0 0x3 has no meaning!\n");
+goto bad_ste;
+}
+
+/* This is still here as stage 2 has not been fully enabled yet. */
 qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
 goto bad_ste;
 }
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index 1e666e8b6d..7906e359d9 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -28,6 +28,7 @@
 #define SMMU_PCI_DEVFN(sid)   (sid & 0xFF)
 
 #define SMMU_MAX_VA_BITS  48
+#define SMMU_MAX_IPA_BITS 48
 #define SMMU_MAX_LEVELS   4
 
 /*
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 16/16] hw/arm/smmuv3: Enable stage-2 support

2023-02-05 Thread Mostafa Saleh
As everything is in place, we can use the new system property to
advertise which stage is supported and remove bad_ste from STE
stage2 config.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmuv3.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 5f792d96ab..4b66760104 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -256,7 +256,14 @@ static void smmuv3_init_regs(SMMUv3State *s)
  * IDR0: stage1 only, AArch64 only, coherent access, 16b ASID,
  *   multi-level stream table
  */
-s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S1P, 1); /* stage 1 supported */
+if (STAGE1_SUPPORTED(s->features)) {
+s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S1P, 1);
+}
+
+if (STAGE2_SUPPORTED(s->features)) {
+s->idr[0] = FIELD_DP32(s->idr[0], IDR0, S2P, 1);
+}
+
 s->idr[0] = FIELD_DP32(s->idr[0], IDR0, TTF, 2); /* AArch64 PTW only */
 s->idr[0] = FIELD_DP32(s->idr[0], IDR0, COHACC, 1); /* IO coherent */
 s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ASID16, 1); /* 16-bit ASID */
@@ -451,10 +458,6 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
 qemu_log_mask(LOG_UNIMP, "SMMUv3 Stall not implemented!\n");
 goto bad_ste;
 }
-
-/* This is still here as stage 2 has not been fully enabled yet. */
-qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
-goto bad_ste;
 }
 
 if (STE_S1CDMAX(ste) != 0) {
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 10/16] hw/arm/smmuv3: Make TLB lookup work for stage-2

2023-02-05 Thread Mostafa Saleh
Right now, either stage-1 or stage-2 are supported, this simplifies
how we can deal with TLBs.
This patch makes TLB lookup work if stage-2 is enabled instead of
stage-1.
TLB lookup is done before a PTW, if a valid entry is found we won't
do the PTW.
To be able to do TLB lookup, we need the correct tagging info, as
granularity and input size, so we get this based on the supported
translation stage. The TLB entries are added correctly from each
stage PTW.

When nested translation is supported, this would need to change, for
example if we go with a combined TLB implementation, we would need to
use the min of the granularities in TLB.

As stage-2 shouldn't be tagged by ASID, it will be set to -1 if S1P
is not enabled.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmuv3.c | 44 +---
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index c18460a4ff..769c735697 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -653,6 +653,8 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, 
SMMUTransCfg *cfg,
 return ret;
 }
 
+/* ASID defaults to -1 if s1 is not supported. */
+cfg->asid = -1;
 if (cfg->aborted || cfg->bypassed || !STAGE1_SUPPORTED(s->features)) {
 return 0;
 }
@@ -733,6 +735,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion 
*mr, hwaddr addr,
 SMMUTLBEntry *cached_entry = NULL;
 SMMUTransTableInfo *tt;
 SMMUTransCfg *cfg = NULL;
+uint8_t granule_sz, tsz;
 IOMMUTLBEntry entry = {
 .target_as = &address_space_memory,
 .iova = addr,
@@ -764,21 +767,40 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion 
*mr, hwaddr addr,
 goto epilogue;
 }
 
-tt = select_tt(cfg, addr);
-if (!tt) {
-if (cfg->record_faults) {
-event.type = SMMU_EVT_F_TRANSLATION;
-event.u.f_translation.addr = addr;
-event.u.f_translation.rnw = flag & 0x1;
+if (STAGE1_SUPPORTED(s->features)) {
+/* Select stage1 translation table. */
+tt = select_tt(cfg, addr);
+if (!tt) {
+if (cfg->record_faults) {
+event.type = SMMU_EVT_F_TRANSLATION;
+event.u.f_translation.addr = addr;
+event.u.f_translation.rnw = flag & 0x1;
+}
+status = SMMU_TRANS_ERROR;
+goto epilogue;
 }
-status = SMMU_TRANS_ERROR;
-goto epilogue;
-}
+granule_sz = tt->granule_sz;
+tsz = tt->tsz;
 
-page_mask = (1ULL << (tt->granule_sz)) - 1;
+} else {
+/* Stage2. */
+granule_sz = cfg->s2cfg.granule_sz;
+tsz = cfg->s2cfg.tsz;
+}
+/*
+ * TLB lookup looks for granule and input size for a translation stage,
+ * as only one stage is supported right now, choose the right values
+ * from the configuration.
+ */
+page_mask = (1ULL << granule_sz) - 1;
 aligned_addr = addr & ~page_mask;
 
-cached_entry = smmu_iotlb_lookup(bs, cfg, tt, aligned_addr);
+SMMUTransTableInfo temp = {
+.granule_sz = granule_sz,
+.tsz = tsz,
+};
+
+cached_entry = smmu_iotlb_lookup(bs, cfg, &temp, aligned_addr);
 if (cached_entry) {
 if ((flag & IOMMU_WO) && !(cached_entry->entry.perm & IOMMU_WO)) {
 status = SMMU_TRANS_ERROR;
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 15/16] hw/arm/smmuv3: Add fault configuration for stage-2

2023-02-05 Thread Mostafa Saleh
As stall is not supported, if S2S is set the translation would abort.
For S2R, we reuse the same code used for stage-1 with flag
record_faults. However when nested translation is supported we would
need to separate stage-1 and stage-2 faults.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmuv3-internal.h | 2 ++
 hw/arm/smmuv3.c  | 5 +
 2 files changed, 7 insertions(+)

diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index 3ccb9d118e..ccdae81db8 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -528,6 +528,8 @@ typedef struct CD {
 #define STE_S2HD(x)extract32((x)->word[5], 24, 1)
 #define STE_S2HA(x)extract32((x)->word[5], 25, 1)
 #define STE_S2S(x) extract32((x)->word[5], 26, 1)
+#define STE_S2R(x) extract32((x)->word[5], 27, 1)
+
 #define STE_CTXPTR(x)   \
 ({  \
 unsigned long addr; \
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index e0976ac236..5f792d96ab 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -446,6 +446,11 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
 }
 
 cfg->s2cfg.affd = STE_S2AFFD(ste);
+cfg->record_faults = STE_S2R(ste);
+if (STE_S2S(ste)) {
+qemu_log_mask(LOG_UNIMP, "SMMUv3 Stall not implemented!\n");
+goto bad_ste;
+}
 
 /* This is still here as stage 2 has not been fully enabled yet. */
 qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 02/16] hw/arm/smmuv3: Update translation config to hold stage-2

2023-02-05 Thread Mostafa Saleh
In preparation for adding stage-2 support. Add it's configuration.

They are added as SMMUS2Cfg in SMMUTransCfg, SMMUS2Cfg hold configs
parsed from STE:
 -tsz: Input range
 -sl0: start level of translation
 -affd: AF fault disable
 -granule_sz: Granule page shift
 -vmid: VMID
 -vttb: PA of translation table

They will be used in the next patches in stage-2 address translation.

No functional change intended.

Signed-off-by: Mostafa Saleh 
---
 include/hw/arm/smmu-common.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index c5683af07d..45f74d0e93 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -60,6 +60,16 @@ typedef struct SMMUTLBEntry {
 uint8_t granule;
 } SMMUTLBEntry;
 
+typedef struct SMMUS2Cfg {
+uint8_t tsz;/* Input range */
+uint8_t sl0;/* Start level of translation */
+bool affd;  /* AF Fault Disable */
+uint8_t granule_sz; /* Granule page shift */
+uint16_t vmid;  /* Virtual machine ID */
+uint64_t vttb;  /* PA of translation table */
+} SMMUS2Cfg;
+
+
 /*
  * Generic structure populated by derived SMMU devices
  * after decoding the configuration information and used as
@@ -79,6 +89,7 @@ typedef struct SMMUTransCfg {
 SMMUTransTableInfo tt[2];
 uint32_t iotlb_hits;   /* counts IOTLB hits for this asid */
 uint32_t iotlb_misses; /* counts IOTLB misses for this asid */
+struct SMMUS2Cfg s2cfg;
 } SMMUTransCfg;
 
 typedef struct SMMUDevice {
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 12/16] hw/arm/smmuv3: Add VMID to tlb tagging

2023-02-05 Thread Mostafa Saleh
Allow TLB to be tagged with VMID.

If stage-1 is only supported, VMID is set to -1 and ignored from STE
and CMD_TLBI_NH* cmds.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmu-common.c | 24 +++-
 hw/arm/smmu-internal.h   |  2 ++
 hw/arm/smmuv3.c  | 12 +---
 include/hw/arm/smmu-common.h |  5 +++--
 4 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 541c427684..028a60949a 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -56,10 +56,11 @@ static gboolean smmu_iotlb_key_equal(gconstpointer v1, 
gconstpointer v2)
(k1->level == k2->level) && (k1->tg == k2->tg);
 }
 
-SMMUIOTLBKey smmu_get_iotlb_key(uint16_t asid, uint64_t iova,
+SMMUIOTLBKey smmu_get_iotlb_key(uint16_t asid, uint16_t vmid, uint64_t iova,
 uint8_t tg, uint8_t level)
 {
-SMMUIOTLBKey key = {.asid = asid, .iova = iova, .tg = tg, .level = level};
+SMMUIOTLBKey key = {.asid = asid, .vmid = vmid, .iova = iova,
+.tg = tg, .level = level};
 
 return key;
 }
@@ -78,7 +79,8 @@ SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, SMMUTransCfg 
*cfg,
 uint64_t mask = subpage_size - 1;
 SMMUIOTLBKey key;
 
-key = smmu_get_iotlb_key(cfg->asid, iova & ~mask, tg, level);
+key = smmu_get_iotlb_key(cfg->asid, cfg->s2cfg.vmid,
+ iova & ~mask, tg, level);
 entry = g_hash_table_lookup(bs->iotlb, &key);
 if (entry) {
 break;
@@ -111,7 +113,8 @@ void smmu_iotlb_insert(SMMUState *bs, SMMUTransCfg *cfg, 
SMMUTLBEntry *new)
 smmu_iotlb_inv_all(bs);
 }
 
-*key = smmu_get_iotlb_key(cfg->asid, new->entry.iova, tg, new->level);
+*key = smmu_get_iotlb_key(cfg->asid, cfg->s2cfg.vmid, new->entry.iova,
+  tg, new->level);
 trace_smmu_iotlb_insert(cfg->asid, new->entry.iova, tg, new->level);
 g_hash_table_insert(bs->iotlb, key, new);
 }
@@ -130,8 +133,7 @@ static gboolean smmu_hash_remove_by_asid(gpointer key, 
gpointer value,
 
 return SMMU_IOTLB_ASID(*iotlb_key) == asid;
 }
-
-static gboolean smmu_hash_remove_by_asid_iova(gpointer key, gpointer value,
+static gboolean smmu_hash_remove_by_asid_vmid_iova(gpointer key, gpointer 
value,
   gpointer user_data)
 {
 SMMUTLBEntry *iter = (SMMUTLBEntry *)value;
@@ -142,18 +144,21 @@ static gboolean smmu_hash_remove_by_asid_iova(gpointer 
key, gpointer value,
 if (info->asid >= 0 && info->asid != SMMU_IOTLB_ASID(iotlb_key)) {
 return false;
 }
+if (info->vmid >= 0 && info->vmid != SMMU_IOTLB_VMID(iotlb_key)) {
+return false;
+}
 return ((info->iova & ~entry->addr_mask) == entry->iova) ||
((entry->iova & ~info->mask) == info->iova);
 }
 
-void smmu_iotlb_inv_iova(SMMUState *s, int asid, dma_addr_t iova,
+void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova,
  uint8_t tg, uint64_t num_pages, uint8_t ttl)
 {
 /* if tg is not set we use 4KB range invalidation */
 uint8_t granule = tg ? tg * 2 + 10 : 12;
 
 if (ttl && (num_pages == 1) && (asid >= 0)) {
-SMMUIOTLBKey key = smmu_get_iotlb_key(asid, iova, tg, ttl);
+SMMUIOTLBKey key = smmu_get_iotlb_key(asid, vmid, iova, tg, ttl);
 
 if (g_hash_table_remove(s->iotlb, &key)) {
 return;
@@ -166,10 +171,11 @@ void smmu_iotlb_inv_iova(SMMUState *s, int asid, 
dma_addr_t iova,
 
 SMMUIOTLBPageInvInfo info = {
 .asid = asid, .iova = iova,
+.vmid = vmid,
 .mask = (num_pages * 1 << granule) - 1};
 
 g_hash_table_foreach_remove(s->iotlb,
-smmu_hash_remove_by_asid_iova,
+smmu_hash_remove_by_asid_vmid_iova,
 &info);
 }
 
diff --git a/hw/arm/smmu-internal.h b/hw/arm/smmu-internal.h
index 7d3f76ce14..3a14e5dca5 100644
--- a/hw/arm/smmu-internal.h
+++ b/hw/arm/smmu-internal.h
@@ -136,9 +136,11 @@ static inline int pgd_idx(int start_level, int granule, 
dma_addr_t iova)
 }
 
 #define SMMU_IOTLB_ASID(key) ((key).asid)
+#define SMMU_IOTLB_VMID(key) ((key).vmid)
 
 typedef struct SMMUIOTLBPageInvInfo {
 int asid;
+int vmid;
 uint64_t iova;
 uint64_t mask;
 } SMMUIOTLBPageInvInfo;
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 35a0149bbf..8b070f6bb5 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -986,7 +986,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
 {
 dma_addr_t end, addr = CMD_ADDR(cmd);
 uint8_t type = CMD_TYPE(cmd);
-uint16_t vmid = CMD_VMID(cmd);
+int vmid = -1;
 uint8_t scale = CMD_SCALE(cmd);
 uint8_t num = CMD_NUM(cmd);
 uint8_t ttl = CMD_TTL(cmd);
@@ -995,6 +995,12 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
 uint64_t num_pages;
 uint8_t granul

[RFC PATCH 11/16] hw/arm/smmuv3: Read VMID from STE

2023-02-05 Thread Mostafa Saleh
According to SMMUv3 user manual "5.2 Stream Table Entry": All fields
with an S2 prefix (with the exception of S2VMID) are IGNORED when
stage-2 bypasses translation (Config[1] == 0).

Which means that VMID can be used(for TLB tagging) even if stage-2 is
bypassed, so we parse it unconditionally when S2P exists. Otherwise
it is set to -1.(only S1P)

Advertise 16-bit VMID is supported.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmuv3.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 769c735697..35a0149bbf 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -260,6 +260,7 @@ static void smmuv3_init_regs(SMMUv3State *s)
 s->idr[0] = FIELD_DP32(s->idr[0], IDR0, TTF, 2); /* AArch64 PTW only */
 s->idr[0] = FIELD_DP32(s->idr[0], IDR0, COHACC, 1); /* IO coherent */
 s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ASID16, 1); /* 16-bit ASID */
+s->idr[0] = FIELD_DP32(s->idr[0], IDR0, VMID16, 1); /* 16-bit VMID */
 s->idr[0] = FIELD_DP32(s->idr[0], IDR0, TTENDIAN, 2); /* little endian */
 s->idr[0] = FIELD_DP32(s->idr[0], IDR0, STALL_MODEL, 1); /* No stall */
 /* terminated transaction will always be aborted/error returned */
@@ -388,6 +389,14 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
 return 0;
 }
 
+if (STAGE2_SUPPORTED(s->features)) {
+/* VMID is considered even if s2 is disabled. */
+cfg->s2cfg.vmid = STE_S2VMID(ste);
+} else {
+/* Default to -1 */
+cfg->s2cfg.vmid = -1;
+}
+
 if (STAGE2_SUPPORTED(s->features) && STE_CFG_S2_ENABLED(config)) {
 cfg->stage = 2;
 
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 14/16] hw/arm/smmuv3: Add stage-2 support in iova notifier

2023-02-05 Thread Mostafa Saleh
In smmuv3_notify_iova, read the granule based on translation stage
and use VMID if valid value is sent.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmuv3.c | 39 ++-
 hw/arm/trace-events |  2 +-
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 2b563a5b1b..e0976ac236 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -919,18 +919,21 @@ epilogue:
  * @mr: IOMMU mr region handle
  * @n: notifier to be called
  * @asid: address space ID or negative value if we don't care
+ * @vmid: virtual machine ID or negative value if we don't care
  * @iova: iova
  * @tg: translation granule (if communicated through range invalidation)
  * @num_pages: number of @granule sized pages (if tg != 0), otherwise 1
  */
 static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
IOMMUNotifier *n,
-   int asid, dma_addr_t iova,
-   uint8_t tg, uint64_t num_pages)
+   int asid, int vmid,
+   dma_addr_t iova, uint8_t tg,
+   uint64_t num_pages)
 {
 SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
 IOMMUTLBEvent event;
 uint8_t granule;
+SMMUv3State *s = sdev->smmu;
 
 if (!tg) {
 SMMUEventInfo event = {.inval_ste_allowed = true};
@@ -945,11 +948,20 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
 return;
 }
 
-tt = select_tt(cfg, iova);
-if (!tt) {
+if (vmid >= 0 && cfg->s2cfg.vmid != vmid) {
 return;
 }
-granule = tt->granule_sz;
+
+if (STAGE1_SUPPORTED(s->features)) {
+tt = select_tt(cfg, iova);
+if (!tt) {
+return;
+}
+granule = tt->granule_sz;
+} else {
+granule = cfg->s2cfg.granule_sz;
+}
+
 } else {
 granule = tg * 2 + 10;
 }
@@ -963,9 +975,10 @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,
 memory_region_notify_iommu_one(n, &event);
 }
 
-/* invalidate an asid/iova range tuple in all mr's */
-static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, dma_addr_t iova,
-  uint8_t tg, uint64_t num_pages)
+/* invalidate an asid/vmid/iova range tuple in all mr's */
+static void smmuv3_inv_notifiers_iova(SMMUState *s, int asid, int vmid,
+  dma_addr_t iova, uint8_t tg,
+  uint64_t num_pages)
 {
 SMMUDevice *sdev;
 
@@ -973,11 +986,11 @@ static void smmuv3_inv_notifiers_iova(SMMUState *s, int 
asid, dma_addr_t iova,
 IOMMUMemoryRegion *mr = &sdev->iommu;
 IOMMUNotifier *n;
 
-trace_smmuv3_inv_notifiers_iova(mr->parent_obj.name, asid, iova,
-tg, num_pages);
+trace_smmuv3_inv_notifiers_iova(mr->parent_obj.name, asid, vmid,
+iova, tg, num_pages);
 
 IOMMU_NOTIFIER_FOREACH(n, mr) {
-smmuv3_notify_iova(mr, n, asid, iova, tg, num_pages);
+smmuv3_notify_iova(mr, n, asid, vmid, iova, tg, num_pages);
 }
 }
 }
@@ -1008,7 +1021,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
 
 if (!tg) {
 trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, 1, ttl, leaf);
-smmuv3_inv_notifiers_iova(s, asid, addr, tg, 1);
+smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, 1);
 smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, 1, ttl);
 return;
 }
@@ -1026,7 +1039,7 @@ static void smmuv3_s1_range_inval(SMMUState *s, Cmd *cmd)
 
 num_pages = (mask + 1) >> granule;
 trace_smmuv3_s1_range_inval(vmid, asid, addr, tg, num_pages, ttl, 
leaf);
-smmuv3_inv_notifiers_iova(s, asid, addr, tg, num_pages);
+smmuv3_inv_notifiers_iova(s, asid, vmid, addr, tg, num_pages);
 smmu_iotlb_inv_iova(s, asid, vmid, addr, tg, num_pages, ttl);
 addr += mask + 1;
 }
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index 61e2ffade5..625cf16f69 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -53,5 +53,5 @@ smmuv3_cmdq_tlbi_s12_vmid(uint16_t vmid) "vmid=%d"
 smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid=0x%x"
 smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu 
mr=%s"
 smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu 
mr=%s"
-smmuv3_inv_notifiers_iova(const char *name, uint16_t asid, uint64_t iova, 
uint8_t tg, uint64_t num_pages) "iommu mr=%s asid=%d iova=0x%"PRIx64" tg=%d 
num_pages=0x%"PRIx64
+smmuv3_inv_notifiers_iova(const char *name, uint16_t asid, uint16_t vmid, 
uint64_t iova, uint8_t tg, uint64_t num_pages) "iommu mr=%s asid=%d vmid=%d 
iova=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64
 
-- 
2.39.1.519.gcb327c4b5f-goog




[RFC PATCH 05/16] hw/arm/smmuv3: Add page table walk for stage-2

2023-02-05 Thread Mostafa Saleh
In preparation for adding stage-2 support. Add Stage-2 PTW code.
Only Aarch64 fromat is supported as stage-1.
Max 48 bits IPA is supported.

Nesting stage-1 and stage-2 is not supported right now.

Signed-off-by: Mostafa Saleh 
---
 hw/arm/smmu-common.c | 112 ---
 hw/arm/smmu-internal.h   |  37 
 include/hw/arm/smmu-common.h |   1 +
 3 files changed, 143 insertions(+), 7 deletions(-)

diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 4fcbffa2f1..df0d1dc024 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -362,6 +362,99 @@ error:
 return -EINVAL;
 }
 
+/**
+ * smmu_ptw_64_s2 - VMSAv8-64 Walk of the page tables for a given IOVA
+ * for stage-2.
+ * @cfg: translation config
+ * @iova: iova to translate
+ * @perm: access type
+ * @tlbe: SMMUTLBEntry (out)
+ * @info: handle to an error info
+ *
+ * Return 0 on success, < 0 on error. In case of error, @info is filled
+ * and tlbe->perm is set to IOMMU_NONE.
+ * Upon success, @tlbe is filled with translated_addr and entry
+ * permission rights.
+ */
+
+static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
+  dma_addr_t iova, IOMMUAccessFlags perm,
+  SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
+{
+const int stage = 2;
+int granule_sz = cfg->s2cfg.granule_sz;
+/* ARM ARM: Table D8-7. */
+int inputsize = 64 - cfg->s2cfg.tsz;
+int level = get_start_level(cfg->s2cfg.sl0, granule_sz);
+int stride = granule_sz - 3;
+int idx = pgd_idx(level, granule_sz, iova);
+/*
+ * Get the ttb from concatenated structure.
+ * The offset is the idx * size of each ttb(number of ptes * (sizeof(pte))
+ */
+uint64_t baseaddr = extract64(cfg->s2cfg.vttb, 0, 48) + (1 << stride) *
+idx * sizeof(uint64_t);
+dma_addr_t indexmask = (1ULL << (inputsize - (stride * (4 - level - 1;
+
+baseaddr &= ~indexmask;
+
+while (level < SMMU_MAX_LEVELS) {
+uint64_t subpage_size = 1ULL << level_shift(level, granule_sz);
+uint64_t mask = subpage_size - 1;
+uint32_t offset = iova_level_offset(iova, inputsize, level, 
granule_sz);
+uint64_t pte, gpa;
+dma_addr_t pte_addr = baseaddr + offset * sizeof(pte);
+uint8_t ap;
+
+if (get_pte(baseaddr, offset, &pte, info)) {
+goto error;
+}
+trace_smmu_ptw_level(level, iova, subpage_size,
+ baseaddr, offset, pte);
+if (is_invalid_pte(pte) || is_reserved_pte(pte, level)) {
+trace_smmu_ptw_invalid_pte(stage, level, baseaddr,
+   pte_addr, offset, pte);
+break;
+}
+
+if (is_table_pte(pte, level)) {
+baseaddr = get_table_pte_address(pte, granule_sz);
+level++;
+continue;
+} else if (is_page_pte(pte, level)) {
+gpa = get_page_pte_address(pte, granule_sz);
+trace_smmu_ptw_page_pte(stage, level, iova,
+baseaddr, pte_addr, pte, gpa);
+} else {
+uint64_t block_size;
+
+gpa = get_block_pte_address(pte, level, granule_sz,
+&block_size);
+trace_smmu_ptw_block_pte(stage, level, baseaddr,
+ pte_addr, pte, iova, gpa,
+ block_size >> 20);
+}
+ap = PTE_AP(pte);
+if (is_permission_fault_s2(ap, perm)) {
+info->type = SMMU_PTW_ERR_PERMISSION;
+goto error;
+}
+
+tlbe->entry.translated_addr = gpa;
+tlbe->entry.iova = iova & ~mask;
+tlbe->entry.addr_mask = mask;
+tlbe->entry.perm = ap;
+tlbe->level = level;
+tlbe->granule = granule_sz;
+return 0;
+}
+info->type = SMMU_PTW_ERR_TRANSLATION;
+
+error:
+tlbe->entry.perm = IOMMU_NONE;
+return -EINVAL;
+}
+
 /**
  * smmu_ptw - Walk the page tables for an IOVA, according to @cfg
  *
@@ -376,15 +469,20 @@ error:
 int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
  SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
 {
-if (!cfg->aa64) {
-/*
- * This code path is not entered as we check this while decoding
- * the configuration data in the derived SMMU model.
- */
-g_assert_not_reached();
+if (cfg->stage == 1) {
+if (!cfg->aa64) {
+/*
+ * This code path is not entered as we check this while decoding
+ * the configuration data in the derived SMMU model.
+ */
+g_assert_not_reached();
+}
+return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
+} else if (cfg->stage == 2) {
+return smmu_ptw_64_s2(cfg, iova, perm, tlbe, info);
 }
 
-return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
+g_assert_n

Re: [RFC v2 12/13] vdpa: preemptive kick at enable

2023-02-05 Thread Michael S. Tsirkin
On Sat, Feb 04, 2023 at 03:04:02AM -0800, Si-Wei Liu wrote:
> For network hardware device, I thought suspend
> just needs to wait until the completion of ongoing Tx/Rx DMA transaction
> already in the flight, rather than to drain all the upcoming packets until
> avail_idx.

It depends I guess but if device expects to recover all state from just
ring state in memory then at least it has to drain until some index
value.




[PULL 03/16] ppc/pegasos2: Improve readability of VIA south bridge creation

2023-02-05 Thread Daniel Henrique Barboza
From: BALATON Zoltan 

Slightly improve readability of creating the south btidge by cnamging
type of a local variable to avoid some casts within function arguments
which makes some lines shorter and easier to read.
Also remove an unneded line break.

Signed-off-by: BALATON Zoltan 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20230117214545.5e191746...@zero.eik.bme.hu>
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/pegasos2.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index f46d4bf51d..1a13632ba6 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -102,7 +102,8 @@ static void pegasos2_init(MachineState *machine)
 CPUPPCState *env;
 MemoryRegion *rom = g_new(MemoryRegion, 1);
 PCIBus *pci_bus;
-PCIDevice *dev, *via;
+Object *via;
+PCIDevice *dev;
 I2CBus *i2c_bus;
 const char *fwname = machine->firmware ?: PROM_FILENAME;
 char *filename;
@@ -159,19 +160,18 @@ static void pegasos2_init(MachineState *machine)
 pci_bus = mv64361_get_pci_bus(pm->mv, 1);
 
 /* VIA VT8231 South Bridge (multifunction PCI device) */
-via = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0), true,
-  TYPE_VT8231_ISA);
+via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0),
+ true, TYPE_VT8231_ISA));
 object_property_add_alias(OBJECT(machine), "rtc-time",
-  object_resolve_path_component(OBJECT(via),
-"rtc"),
+  object_resolve_path_component(via, "rtc"),
   "date");
 qdev_connect_gpio_out(DEVICE(via), 0,
   qdev_get_gpio_in_named(pm->mv, "gpp", 31));
 
-dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "ide"));
+dev = PCI_DEVICE(object_resolve_path_component(via, "ide"));
 pci_ide_create_devs(dev);
 
-dev = PCI_DEVICE(object_resolve_path_component(OBJECT(via), "pm"));
+dev = PCI_DEVICE(object_resolve_path_component(via, "pm"));
 i2c_bus = I2C_BUS(qdev_get_child_bus(DEVICE(dev), "i2c"));
 spd_data = spd_data_generate(DDR, machine->ram_size);
 smbus_eeprom_init_one(i2c_bus, 0x57, spd_data);
-- 
2.39.1




[PULL 04/16] hw/pci-host/mv64361: Reuse pci_swizzle_map_irq_fn

2023-02-05 Thread Daniel Henrique Barboza
From: Bernhard Beschow 

mv64361_pcihost_map_irq() is a reimplementation of
pci_swizzle_map_irq_fn(). Resolve this redundancy.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: BALATON Zoltan 
Message-Id: <20230106113927.8603-1-shen...@gmail.com>
Signed-off-by: Daniel Henrique Barboza 
---
 hw/pci-host/mv64361.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/hw/pci-host/mv64361.c b/hw/pci-host/mv64361.c
index 015b92bd5f..f43f33fbd9 100644
--- a/hw/pci-host/mv64361.c
+++ b/hw/pci-host/mv64361.c
@@ -72,11 +72,6 @@ struct MV64361PCIState {
 uint64_t remap[5];
 };
 
-static int mv64361_pcihost_map_irq(PCIDevice *pci_dev, int n)
-{
-return (n + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS;
-}
-
 static void mv64361_pcihost_set_irq(void *opaque, int n, int level)
 {
 MV64361PCIState *s = opaque;
@@ -97,7 +92,7 @@ static void mv64361_pcihost_realize(DeviceState *dev, Error 
**errp)
 g_free(name);
 name = g_strdup_printf("pci.%d", s->index);
 h->bus = pci_register_root_bus(dev, name, mv64361_pcihost_set_irq,
-   mv64361_pcihost_map_irq, dev,
+   pci_swizzle_map_irq_fn, dev,
&s->mem, &s->io, 0, 4, TYPE_PCI_BUS);
 g_free(name);
 pci_create_simple(h->bus, 0, TYPE_MV64361_PCI_BRIDGE);
-- 
2.39.1




[PULL 16/16] hw/display/sm501: Code style fix

2023-02-05 Thread Daniel Henrique Barboza
From: BALATON Zoltan 

Fix checkpatch warning about multi-line comment.

Signed-off-by: BALATON Zoltan 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: 
<8801292992a304609e1eac680fe36b515592b926.1674333199.git.bala...@eik.bme.hu>
Signed-off-by: Daniel Henrique Barboza 
---
 hw/display/sm501.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 1e17072452..e1d0591d36 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1768,7 +1768,8 @@ static const GraphicHwOps sm501_ops = {
 static void sm501_reset(SM501State *s)
 {
 s->system_control = 0x0010; /* 2D engine FIFO empty */
-/* Bits 17 (SH), 7 (CDR), 6:5 (Test), 2:0 (Bus) are all supposed
+/*
+ * Bits 17 (SH), 7 (CDR), 6:5 (Test), 2:0 (Bus) are all supposed
  * to be determined at reset by GPIO lines which set config bits.
  * We hardwire them:
  *  SH = 0 : Hitachi Ready Polarity == Active Low
-- 
2.39.1




[PULL 00/16] ppc queue

2023-02-05 Thread Daniel Henrique Barboza
The following changes since commit ceabf6e500570ecfb311d8896c4ba9da8cf21f2a:

  Merge tag 'linux-user-for-8.0-pull-request' of 
https://gitlab.com/laurent_vivier/qemu into staging (2023-02-04 17:17:15 +)

are available in the Git repository at:

  https://gitlab.com/danielhb/qemu.git tags/pull-ppc-20230205

for you to fetch changes up to bd591dc1b3c39b7f73b8d9f20be6e9001c905238:

  hw/display/sm501: Code style fix (2023-02-05 06:40:28 -0300)


ppc patch queue for 2023-02-05:

This queue includes patches that aren't PPC specific but benefit/impact
PPC machines, such as the changes to guestperf.py, mv64361 and sm501. As
for PPC specific changes we have e500 and PNV_PHB5 fixes.


BALATON Zoltan (5):
  ppc/pegasos2: Improve readability of VIA south bridge creation
  hw/ppc/pegasos2: Fix a typo in a comment
  hw/display/sm501: Remove parenthesis around constant macro definitions
  hw/display/sm501: Remove unneeded casts from void pointer
  hw/display/sm501: Code style fix

Bernhard Beschow (5):
  hw/pci-host/mv64361: Reuse pci_swizzle_map_irq_fn
  hw/ppc: Set machine->fdt in e500 machines
  hw/ppc/e500{, plat}: Drop redundant checks for presence of platform bus
  hw/ppc/e500.c: Avoid hardcoding parent device in create_devtree_etsec()
  hw/ppc/e500.c: Attach eSDHC unimplemented region to ccsr_addr_space

Frederic Barrat (4):
  ppc/pnv/pci: Cleanup PnvPHBPecState structure
  ppc/pnv/pci: Remove duplicate definition of PNV_PHB5_DEVICE_ID
  ppc/pnv/pci: Update PHB5 version register
  ppc/pnv/pci: Fix PHB xscom registers memory region name

Murilo Opsfelder Araujo (2):
  tests/migration: add sysprof-capture-4 as dependency for stress binary
  tests/migration: add support for ppc64le for guestperf.py

 hw/display/sm501.c  | 419 ++--
 hw/pci-host/mv64361.c   |   7 +-
 hw/pci-host/pnv_phb4.c  |   2 +-
 hw/ppc/e500.c   |  24 ++-
 hw/ppc/e500plat.c   |   9 +-
 hw/ppc/pegasos2.c   |  16 +-
 include/hw/pci-host/pnv_phb4.h  |   5 +-
 tests/migration/guestperf/engine.py |  28 ++-
 tests/migration/meson.build |   4 +-
 9 files changed, 268 insertions(+), 246 deletions(-)



[PULL 07/16] hw/ppc/e500.c: Avoid hardcoding parent device in create_devtree_etsec()

2023-02-05 Thread Daniel Henrique Barboza
From: Bernhard Beschow 

The "platform" node is available through data->node, so use that instead
of making assumptions about the parent device.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Daniel Henrique Barboza 
Message-Id: <20230125130024.158721-4-shen...@gmail.com>
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/e500.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 48288c0b41..e3b29d1d97 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -241,7 +241,7 @@ static int create_devtree_etsec(SysBusDevice *sbdev, 
PlatformDevtreeData *data)
 int irq0 = platform_bus_get_irqn(pbus, sbdev, 0);
 int irq1 = platform_bus_get_irqn(pbus, sbdev, 1);
 int irq2 = platform_bus_get_irqn(pbus, sbdev, 2);
-gchar *node = g_strdup_printf("/platform/ethernet@%"PRIx64, mmio0);
+gchar *node = g_strdup_printf("%s/ethernet@%"PRIx64, data->node, mmio0);
 gchar *group = g_strdup_printf("%s/queue-group", node);
 void *fdt = data->fdt;
 
-- 
2.39.1




[PULL 11/16] ppc/pnv/pci: Update PHB5 version register

2023-02-05 Thread Daniel Henrique Barboza
From: Frederic Barrat 

Update register value per its P10 DD2 definition.

Signed-off-by: Frederic Barrat 
Reviewed-by: Cédric Le Goater 
Message-Id: <20230127122848.550083-4-fbar...@linux.ibm.com>
Signed-off-by: Daniel Henrique Barboza 
---
 include/hw/pci-host/pnv_phb4.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
index 761525686e..28d61b96c7 100644
--- a/include/hw/pci-host/pnv_phb4.h
+++ b/include/hw/pci-host/pnv_phb4.h
@@ -214,7 +214,7 @@ struct PnvPhb4PecClass {
 #define PNV_PHB5(obj) \
 OBJECT_CHECK(PnvPhb4, (obj), TYPE_PNV_PHB5)
 
-#define PNV_PHB5_VERSION   0x00a50001ull
+#define PNV_PHB5_VERSION   0x00a50002ull
 
 #define TYPE_PNV_PHB5_PEC "pnv-phb5-pec"
 #define PNV_PHB5_PEC(obj) \
-- 
2.39.1




[PULL 12/16] ppc/pnv/pci: Fix PHB xscom registers memory region name

2023-02-05 Thread Daniel Henrique Barboza
From: Frederic Barrat 

The name is for the region mapping the PHB xscom registers. It was
apparently a bad cut-and-paste from the per-stack pci xscom area just
above, so we had two regions with the same name.

Signed-off-by: Frederic Barrat 
Reviewed-by: Cédric Le Goater 
Message-Id: <20230127122848.550083-5-fbar...@linux.ibm.com>
Signed-off-by: Daniel Henrique Barboza 
---
 hw/pci-host/pnv_phb4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index ccbde841fc..542f9e2932 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1497,7 +1497,7 @@ static void pnv_phb4_xscom_realize(PnvPHB4 *phb)
   PHB4_PEC_PCI_STK_REGS_COUNT);
 
 /* PHB pass-through */
-snprintf(name, sizeof(name), "xscom-pec-%d.%d-pci-phb-%d",
+snprintf(name, sizeof(name), "xscom-pec-%d.%d-phb-%d",
  pec->chip_id, pec->index, stack_no);
 pnv_xscom_region_init(&phb->phb_regs_mr, OBJECT(phb),
   &pnv_phb4_xscom_ops, phb, name, 0x40);
-- 
2.39.1




[PULL 01/16] tests/migration: add sysprof-capture-4 as dependency for stress binary

2023-02-05 Thread Daniel Henrique Barboza
From: Murilo Opsfelder Araujo 

`make tests/migration/stress` fails with:

FAILED: tests/migration/stress
cc -m64 -mlittle-endian  -o tests/migration/stress 
tests/migration/stress.p/stress.c.o -Wl,--as-needed -Wl,--no-undefined -pie 
-Wl,--warn-common -Wl,-z,relro -Wl,-z,now -fstack-protector-strong -static 
-pthread -Wl,--start-group -lgthread-2.0 -lglib-2.0 -Wl,--end-group
/usr/bin/ld: 
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../lib64/libglib-2.0.a(gutils.c.o):
 in function `.annobin_gutils.c':
(.text+0x3b4): warning: Using 'getpwuid' in statically linked applications 
requires at runtime the shared libraries from the glibc version used for linking
/usr/bin/ld: (.text+0x178): warning: Using 'getpwnam_r' in statically 
linked applications requires at runtime the shared libraries from the glibc 
version used for linking
/usr/bin/ld: (.text+0x1bc): warning: Using 'getpwuid_r' in statically 
linked applications requires at runtime the shared libraries from the glibc 
version used for linking
/usr/bin/ld: 
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../lib64/libglib-2.0.a(gthread.c.o):(.toc+0x0):
 undefined reference to `sysprof_clock'
/usr/bin/ld: 
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../lib64/libglib-2.0.a(gtrace.c.o):
 in function `.annobin_gtrace.c':
(.text+0x24): undefined reference to `sysprof_collector_mark_vprintf'
/usr/bin/ld: 
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../lib64/libglib-2.0.a(gtrace.c.o):
 in function `g_trace_define_int64_counter':
(.text+0x8c): undefined reference to `sysprof_collector_request_counters'
/usr/bin/ld: (.text+0x108): undefined reference to 
`sysprof_collector_define_counters'
/usr/bin/ld: 
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../lib64/libglib-2.0.a(gtrace.c.o):
 in function `g_trace_set_int64_counter':
(.text+0x23c): undefined reference to `sysprof_collector_set_counters'
/usr/bin/ld: 
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../lib64/libglib-2.0.a(gspawn.c.o):(.toc+0x0):
 undefined reference to `sysprof_clock'
/usr/bin/ld: 
/usr/lib/gcc/ppc64le-redhat-linux/11/../../../../lib64/libglib-2.0.a(gmain.c.o):(.toc+0x0):
 undefined reference to `sysprof_clock'
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.
make: *** [Makefile:162: run-ninja] Error 1

Add sysprof-capture-4 as dependency for stress binary.

Tested on:
  - CentOS Stream 9 ppc64le
  - Fedora 36 x86_64

Signed-off-by: Murilo Opsfelder Araujo 
Reviewed-by: Daniel Henrique Barboza 
Reviewed-by: Juan Quintela 
Message-Id: <20220809002451.91541-2-muri...@linux.ibm.com>
Signed-off-by: Daniel Henrique Barboza 
---
 tests/migration/meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/migration/meson.build b/tests/migration/meson.build
index f215ee7d3a..dd562355a1 100644
--- a/tests/migration/meson.build
+++ b/tests/migration/meson.build
@@ -1,7 +1,9 @@
+sysprof = dependency('sysprof-capture-4', required: false)
+
 stress = executable(
   'stress',
   files('stress.c'),
-  dependencies: [glib],
+  dependencies: [glib, sysprof],
   link_args: ['-static'],
   build_by_default: false,
 )
-- 
2.39.1




[PULL 09/16] ppc/pnv/pci: Cleanup PnvPHBPecState structure

2023-02-05 Thread Daniel Henrique Barboza
From: Frederic Barrat 

Remove unused structure member 'system_memory'.

Signed-off-by: Frederic Barrat 
Reviewed-by: Cédric Le Goater 
Message-Id: <20230127122848.550083-2-fbar...@linux.ibm.com>
Signed-off-by: Daniel Henrique Barboza 
---
 include/hw/pci-host/pnv_phb4.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
index 1f3237c9d5..17aef08f91 100644
--- a/include/hw/pci-host/pnv_phb4.h
+++ b/include/hw/pci-host/pnv_phb4.h
@@ -173,8 +173,6 @@ struct PnvPhb4PecState {
 uint32_t index;
 uint32_t chip_id;
 
-MemoryRegion *system_memory;
-
 /* Nest registers, excuding per-stack */
 #define PHB4_PEC_NEST_REGS_COUNT0xf
 uint64_t nest_regs[PHB4_PEC_NEST_REGS_COUNT];
-- 
2.39.1




[PULL 15/16] hw/display/sm501: Remove unneeded casts from void pointer

2023-02-05 Thread Daniel Henrique Barboza
From: BALATON Zoltan 

This is not needed in C.

Signed-off-by: BALATON Zoltan 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: 
<58f599387dd0739ea1880bfb678872c0be26bf1b.1674333199.git.bala...@eik.bme.hu>
Signed-off-by: Daniel Henrique Barboza 
---
 hw/display/sm501.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 0cbd1fecd5..1e17072452 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -868,7 +868,7 @@ static void sm501_2d_operation(SM501State *s)
 static uint64_t sm501_system_config_read(void *opaque, hwaddr addr,
  unsigned size)
 {
-SM501State *s = (SM501State *)opaque;
+SM501State *s = opaque;
 uint32_t ret = 0;
 
 switch (addr) {
@@ -928,7 +928,7 @@ static uint64_t sm501_system_config_read(void *opaque, 
hwaddr addr,
 static void sm501_system_config_write(void *opaque, hwaddr addr,
   uint64_t value, unsigned size)
 {
-SM501State *s = (SM501State *)opaque;
+SM501State *s = opaque;
 
 trace_sm501_system_config_write((uint32_t)addr, (uint32_t)value);
 switch (addr) {
@@ -996,7 +996,7 @@ static const MemoryRegionOps sm501_system_config_ops = {
 
 static uint64_t sm501_i2c_read(void *opaque, hwaddr addr, unsigned size)
 {
-SM501State *s = (SM501State *)opaque;
+SM501State *s = opaque;
 uint8_t ret = 0;
 
 switch (addr) {
@@ -1023,7 +1023,7 @@ static uint64_t sm501_i2c_read(void *opaque, hwaddr addr, 
unsigned size)
 static void sm501_i2c_write(void *opaque, hwaddr addr, uint64_t value,
 unsigned size)
 {
-SM501State *s = (SM501State *)opaque;
+SM501State *s = opaque;
 
 trace_sm501_i2c_write((uint32_t)addr, (uint32_t)value);
 switch (addr) {
@@ -1092,7 +1092,7 @@ static const MemoryRegionOps sm501_i2c_ops = {
 
 static uint32_t sm501_palette_read(void *opaque, hwaddr addr)
 {
-SM501State *s = (SM501State *)opaque;
+SM501State *s = opaque;
 
 trace_sm501_palette_read((uint32_t)addr);
 
@@ -1106,7 +1106,7 @@ static uint32_t sm501_palette_read(void *opaque, hwaddr 
addr)
 static void sm501_palette_write(void *opaque, hwaddr addr,
 uint32_t value)
 {
-SM501State *s = (SM501State *)opaque;
+SM501State *s = opaque;
 
 trace_sm501_palette_write((uint32_t)addr, value);
 
@@ -1121,7 +1121,7 @@ static void sm501_palette_write(void *opaque, hwaddr addr,
 static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr addr,
  unsigned size)
 {
-SM501State *s = (SM501State *)opaque;
+SM501State *s = opaque;
 uint32_t ret = 0;
 
 switch (addr) {
@@ -1234,7 +1234,7 @@ static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr 
addr,
 static void sm501_disp_ctrl_write(void *opaque, hwaddr addr,
   uint64_t value, unsigned size)
 {
-SM501State *s = (SM501State *)opaque;
+SM501State *s = opaque;
 
 trace_sm501_disp_ctrl_write((uint32_t)addr, (uint32_t)value);
 switch (addr) {
@@ -1379,7 +1379,7 @@ static const MemoryRegionOps sm501_disp_ctrl_ops = {
 static uint64_t sm501_2d_engine_read(void *opaque, hwaddr addr,
  unsigned size)
 {
-SM501State *s = (SM501State *)opaque;
+SM501State *s = opaque;
 uint32_t ret = 0;
 
 switch (addr) {
@@ -1457,7 +1457,7 @@ static uint64_t sm501_2d_engine_read(void *opaque, hwaddr 
addr,
 static void sm501_2d_engine_write(void *opaque, hwaddr addr,
   uint64_t value, unsigned size)
 {
-SM501State *s = (SM501State *)opaque;
+SM501State *s = opaque;
 
 trace_sm501_2d_engine_write((uint32_t)addr, (uint32_t)value);
 switch (addr) {
@@ -1644,7 +1644,7 @@ static void draw_hwc_line_32(uint8_t *d, const uint8_t 
*s, int width,
 
 static void sm501_update_display(void *opaque)
 {
-SM501State *s = (SM501State *)opaque;
+SM501State *s = opaque;
 DisplaySurface *surface = qemu_console_surface(s->con);
 DirtyBitmapSnapshot *snap;
 int y, c_x = 0, c_y = 0;
-- 
2.39.1




[PULL 05/16] hw/ppc: Set machine->fdt in e500 machines

2023-02-05 Thread Daniel Henrique Barboza
From: Bernhard Beschow 

This enables support for the 'dumpdtb' QMP/HMP command for all
e500 machines.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Daniel Henrique Barboza 
Message-Id: <20230125130024.158721-2-shen...@gmail.com>
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/e500.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 9fa1f8e6cf..7239993acc 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -659,9 +659,14 @@ done:
 if (!dry_run) {
 qemu_fdt_dumpdtb(fdt, fdt_size);
 cpu_physical_memory_write(addr, fdt, fdt_size);
+
+/* Set machine->fdt for 'dumpdtb' QMP/HMP command */
+g_free(machine->fdt);
+machine->fdt = fdt;
+} else {
+g_free(fdt);
 }
 ret = fdt_size;
-g_free(fdt);
 
 out:
 g_free(pci_map);
-- 
2.39.1




[PULL 14/16] hw/display/sm501: Remove parenthesis around constant macro definitions

2023-02-05 Thread Daniel Henrique Barboza
From: BALATON Zoltan 

No need to wrap constants in parenthesis.

Signed-off-by: BALATON Zoltan 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: 
<9194546b73b05e7098761ec62b2dfd0699b97b65.1674333199.git.bala...@eik.bme.hu>
Signed-off-by: Daniel Henrique Barboza 
---
 hw/display/sm501.c | 394 ++---
 1 file changed, 197 insertions(+), 197 deletions(-)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 52e42585af..0cbd1fecd5 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -50,10 +50,10 @@
 
 /* System Configuration area */
 /* System config base */
-#define SM501_SYS_CONFIG(0x00)
+#define SM501_SYS_CONFIG0x00
 
 /* config 1 */
-#define SM501_SYSTEM_CONTROL(0x00)
+#define SM501_SYSTEM_CONTROL0x00
 
 #define SM501_SYSCTRL_PANEL_TRISTATE(1 << 0)
 #define SM501_SYSCTRL_MEM_TRISTATE  (1 << 1)
@@ -72,13 +72,13 @@
 
 /* miscellaneous control */
 
-#define SM501_MISC_CONTROL  (0x04)
+#define SM501_MISC_CONTROL  0x04
 
-#define SM501_MISC_BUS_SH   (0x0)
-#define SM501_MISC_BUS_PCI  (0x1)
-#define SM501_MISC_BUS_XSCALE   (0x2)
-#define SM501_MISC_BUS_NEC  (0x6)
-#define SM501_MISC_BUS_MASK (0x7)
+#define SM501_MISC_BUS_SH   0x0
+#define SM501_MISC_BUS_PCI  0x1
+#define SM501_MISC_BUS_XSCALE   0x2
+#define SM501_MISC_BUS_NEC  0x6
+#define SM501_MISC_BUS_MASK 0x7
 
 #define SM501_MISC_VR_62MB  (1 << 3)
 #define SM501_MISC_CDR_RESET(1 << 7)
@@ -103,22 +103,22 @@
 
 
 
-#define SM501_GPIO31_0_CONTROL  (0x08)
-#define SM501_GPIO63_32_CONTROL (0x0C)
-#define SM501_DRAM_CONTROL  (0x10)
+#define SM501_GPIO31_0_CONTROL  0x08
+#define SM501_GPIO63_32_CONTROL 0x0C
+#define SM501_DRAM_CONTROL  0x10
 
 /* command list */
-#define SM501_ARBTRTN_CONTROL   (0x14)
+#define SM501_ARBTRTN_CONTROL   0x14
 
 /* command list */
-#define SM501_COMMAND_LIST_STATUS   (0x24)
+#define SM501_COMMAND_LIST_STATUS   0x24
 
 /* interrupt debug */
-#define SM501_RAW_IRQ_STATUS(0x28)
-#define SM501_RAW_IRQ_CLEAR (0x28)
-#define SM501_IRQ_STATUS(0x2C)
-#define SM501_IRQ_MASK  (0x30)
-#define SM501_DEBUG_CONTROL (0x34)
+#define SM501_RAW_IRQ_STATUS0x28
+#define SM501_RAW_IRQ_CLEAR 0x28
+#define SM501_IRQ_STATUS0x2C
+#define SM501_IRQ_MASK  0x30
+#define SM501_DEBUG_CONTROL 0x34
 
 /* power management */
 #define SM501_POWERMODE_P2X_SRC (1 << 29)
@@ -126,74 +126,74 @@
 #define SM501_POWERMODE_M_SRC   (1 << 12)
 #define SM501_POWERMODE_M1_SRC  (1 << 4)
 
-#define SM501_CURRENT_GATE  (0x38)
-#define SM501_CURRENT_CLOCK (0x3C)
-#define SM501_POWER_MODE_0_GATE (0x40)
-#define SM501_POWER_MODE_0_CLOCK(0x44)
-#define SM501_POWER_MODE_1_GATE (0x48)
-#define SM501_POWER_MODE_1_CLOCK(0x4C)
-#define SM501_SLEEP_MODE_GATE   (0x50)
-#define SM501_POWER_MODE_CONTROL(0x54)
+#define SM501_CURRENT_GATE  0x38
+#define SM501_CURRENT_CLOCK 0x3C
+#define SM501_POWER_MODE_0_GATE 0x40
+#define SM501_POWER_MODE_0_CLOCK0x44
+#define SM501_POWER_MODE_1_GATE 0x48
+#define SM501_POWER_MODE_1_CLOCK0x4C
+#define SM501_SLEEP_MODE_GATE   0x50
+#define SM501_POWER_MODE_CONTROL0x54
 
 /* power gates for units within the 501 */
-#define SM501_GATE_HOST (0)
-#define SM501_GATE_MEMORY   (1)
-#define SM501_GATE_DISPLAY  (2)
-#define SM501_GATE_2D_ENGINE(3)
-#define SM501_GATE_CSC  (4)
-#define SM501_GATE_ZVPORT   (5)
-#define SM501_GATE_GPIO (6)
-#define SM501_GATE_UART0(7)
-#define SM501_GATE_UART1(8)
-#define SM501_GATE_SSP  (10)
-#define SM501_GATE_USB_HOST (11)
-#define SM501_GATE_USB_GADGET   (12)
-#define SM501_GATE_UCONTROLLER  (17)
-#define SM501_GATE_AC97 (18)
+#define SM501_GATE_HOST 0
+#define SM501_GATE_MEMORY   1
+#define SM501_GATE_DISPLAY  2
+#define SM501_GATE_2D_ENGINE3
+#define SM501_GATE_CSC  4
+#define SM501_GATE_ZVPORT   5
+#define SM501_GATE_GPIO 6
+#define SM501_GATE_UART07
+#define SM501_GATE_UART18
+#define SM501_GATE_SSP  10
+#define SM501_GATE_USB_HOST 11
+#define SM501_GATE_USB_GADGET   12
+#defin

[PULL 10/16] ppc/pnv/pci: Remove duplicate definition of PNV_PHB5_DEVICE_ID

2023-02-05 Thread Daniel Henrique Barboza
From: Frederic Barrat 

PNV_PHB5_DEVICE_ID is defined in two different headers. The definition
in hw/pci-host/pnv_phb4.h was left out in a previous rework.

Remaining definition is in hw/pci-host/pnv_phb.h.

Signed-off-by: Frederic Barrat 
Reviewed-by: Cédric Le Goater 
Message-Id: <20230127122848.550083-3-fbar...@linux.ibm.com>
Signed-off-by: Daniel Henrique Barboza 
---
 include/hw/pci-host/pnv_phb4.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
index 17aef08f91..761525686e 100644
--- a/include/hw/pci-host/pnv_phb4.h
+++ b/include/hw/pci-host/pnv_phb4.h
@@ -215,7 +215,6 @@ struct PnvPhb4PecClass {
 OBJECT_CHECK(PnvPhb4, (obj), TYPE_PNV_PHB5)
 
 #define PNV_PHB5_VERSION   0x00a50001ull
-#define PNV_PHB5_DEVICE_ID 0x0652
 
 #define TYPE_PNV_PHB5_PEC "pnv-phb5-pec"
 #define PNV_PHB5_PEC(obj) \
-- 
2.39.1




[PULL 08/16] hw/ppc/e500.c: Attach eSDHC unimplemented region to ccsr_addr_space

2023-02-05 Thread Daniel Henrique Barboza
From: Bernhard Beschow 

Makes the unimplemented region move together with the CCSR address space
if moved by a bootloader. Moving the CCSR address space isn't
implemented yet but this patch is a preparation for it.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20230125130024.158721-5-shen...@gmail.com>
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/e500.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index e3b29d1d97..117c9c08ed 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -1022,9 +1022,13 @@ void ppce500_init(MachineState *machine)
 
 /* eSDHC */
 if (pmc->has_esdhc) {
-create_unimplemented_device("esdhc",
-pmc->ccsrbar_base + 
MPC85XX_ESDHC_REGS_OFFSET,
-MPC85XX_ESDHC_REGS_SIZE);
+dev = qdev_new(TYPE_UNIMPLEMENTED_DEVICE);
+qdev_prop_set_string(dev, "name", "esdhc");
+qdev_prop_set_uint64(dev, "size", MPC85XX_ESDHC_REGS_SIZE);
+s = SYS_BUS_DEVICE(dev);
+sysbus_realize_and_unref(s, &error_fatal);
+memory_region_add_subregion(ccsr_addr_space, MPC85XX_ESDHC_REGS_OFFSET,
+sysbus_mmio_get_region(s, 0));
 
 /*
  * Compatible with:
-- 
2.39.1




[PULL 13/16] hw/ppc/pegasos2: Fix a typo in a comment

2023-02-05 Thread Daniel Henrique Barboza
From: BALATON Zoltan 

Reported-by: Stefan Weil 
Signed-off-by: BALATON Zoltan 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20230203194312.33834745...@zero.eik.bme.hu>
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/pegasos2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 1a13632ba6..a9563f4fb2 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -564,7 +564,7 @@ static void dt_isa(PCIBus *bus, PCIDevice *d, FDTInfo *fi)
 qemu_fdt_setprop_string(fi->fdt, fi->path, "device_type", "isa");
 qemu_fdt_setprop_string(fi->fdt, fi->path, "name", "isa");
 
-/* addional devices */
+/* additional devices */
 g_string_printf(name, "%s/lpt@i3bc", fi->path);
 qemu_fdt_add_subnode(fi->fdt, name->str);
 qemu_fdt_setprop_cell(fi->fdt, name->str, "clock-frequency", 0);
-- 
2.39.1




[PULL 02/16] tests/migration: add support for ppc64le for guestperf.py

2023-02-05 Thread Daniel Henrique Barboza
From: Murilo Opsfelder Araujo 

Add support for ppc64le for guestperf.py. On ppc, console is usually
hvc0 and serial device for pseries machine is spapr-vty.

Signed-off-by: Murilo Opsfelder Araujo 
Reviewed-by: Daniel Henrique Barboza 
Reviewed-by: Juan Quintela 
Message-Id: <20220809002451.91541-3-muri...@linux.ibm.com>
Signed-off-by: Daniel Henrique Barboza 
---
 tests/migration/guestperf/engine.py | 28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/tests/migration/guestperf/engine.py 
b/tests/migration/guestperf/engine.py
index 59fca2c70b..cc06fac592 100644
--- a/tests/migration/guestperf/engine.py
+++ b/tests/migration/guestperf/engine.py
@@ -281,6 +281,26 @@ def _migrate(self, hardware, scenario, src, dst, 
connect_uri):
 resp = src.command("stop")
 paused = True
 
+def _is_ppc64le(self):
+_, _, _, _, machine = os.uname()
+if machine == "ppc64le":
+return True
+return False
+
+def _get_guest_console_args(self):
+if self._is_ppc64le():
+return "console=hvc0"
+else:
+return "console=ttyS0"
+
+def _get_qemu_serial_args(self):
+if self._is_ppc64le():
+return ["-chardev", "stdio,id=cdev0",
+"-device", "spapr-vty,chardev=cdev0"]
+else:
+return ["-chardev", "stdio,id=cdev0",
+"-device", "isa-serial,chardev=cdev0"]
+
 def _get_common_args(self, hardware, tunnelled=False):
 args = [
 "noapic",
@@ -289,8 +309,10 @@ def _get_common_args(self, hardware, tunnelled=False):
 "noreplace-smp",
 "cgroup_disable=memory",
 "pci=noearly",
-"console=ttyS0",
 ]
+
+args.append(self._get_guest_console_args())
+
 if self._debug:
 args.append("debug")
 else:
@@ -308,12 +330,12 @@ def _get_common_args(self, hardware, tunnelled=False):
 "-kernel", self._kernel,
 "-initrd", self._initrd,
 "-append", cmdline,
-"-chardev", "stdio,id=cdev0",
-"-device", "isa-serial,chardev=cdev0",
 "-m", str((hardware._mem * 1024) + 512),
 "-smp", str(hardware._cpus),
 ]
 
+argv.extend(self._get_qemu_serial_args())
+
 if self._debug:
 argv.extend(["-device", "sga"])
 
-- 
2.39.1




[PULL 06/16] hw/ppc/e500{, plat}: Drop redundant checks for presence of platform bus

2023-02-05 Thread Daniel Henrique Barboza
From: Bernhard Beschow 

This is a follow-up on commit 47a0b1dff7e9 'hw/ppc/mpc8544ds: Add
platform bus': Both mpc85xx boards now have a platform bus
unconditionally.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Daniel Henrique Barboza 
Message-Id: <20230125130024.158721-3-shen...@gmail.com>
Signed-off-by: Daniel Henrique Barboza 
---
 hw/ppc/e500.c | 5 ++---
 hw/ppc/e500plat.c | 9 +++--
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 7239993acc..48288c0b41 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -643,9 +643,8 @@ static int ppce500_load_device_tree(PPCE500MachineState 
*pms,
 }
 g_free(soc);
 
-if (pms->pbus_dev) {
-platform_bus_create_devtree(pms, fdt, mpic);
-}
+platform_bus_create_devtree(pms, fdt, mpic);
+
 g_free(mpic);
 
 pmc->fixup_devtree(fdt);
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index 44bf874b0f..3032bd3f6d 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -46,13 +46,10 @@ static void e500plat_machine_device_plug_cb(HotplugHandler 
*hotplug_dev,
 DeviceState *dev, Error **errp)
 {
 PPCE500MachineState *pms = PPCE500_MACHINE(hotplug_dev);
+MachineClass *mc = MACHINE_GET_CLASS(pms);
 
-if (pms->pbus_dev) {
-MachineClass *mc = MACHINE_GET_CLASS(pms);
-
-if (device_is_dynamic_sysbus(mc, dev)) {
-platform_bus_link_device(pms->pbus_dev, SYS_BUS_DEVICE(dev));
-}
+if (device_is_dynamic_sysbus(mc, dev)) {
+platform_bus_link_device(pms->pbus_dev, SYS_BUS_DEVICE(dev));
 }
 }
 
-- 
2.39.1




Re: [PATCH 00/10] Retire Fork-Based Fuzzing

2023-02-05 Thread Philippe Mathieu-Daudé

On 5/2/23 05:29, Alexander Bulekov wrote:


  * Some device do not completely reset their state. This can lead to
non-reproducible crashes. However, in my local tests, most crashes
were reproducible. OSS-Fuzz shouldn't send us reports unless it can
consistently reproduce a crash.


These devices are buggy, hard/cold reset should be reproducible.


  * In theory, the corpus-format should not change, so the existing
corpus-inputs on OSS-Fuzz will transfer to the new reset()-able
fuzzers.





Re: [PATCH 01/10] hw/sparse-mem: clear memory on reset

2023-02-05 Thread Philippe Mathieu-Daudé

On 5/2/23 05:29, Alexander Bulekov wrote:

We use sparse-mem for fuzzing. For long-running fuzzing processes, we
eventually end up with many allocated sparse-mem pages. To avoid this,
clear the allocated pages on system-reset.

Signed-off-by: Alexander Bulekov 
---
  hw/mem/sparse-mem.c | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH 04/10] fuzz/generic-fuzz: add a limit on DMA bytes written

2023-02-05 Thread Philippe Mathieu-Daudé

On 5/2/23 05:29, Alexander Bulekov wrote:

As we have repplaced fork-based fuzzing, with reboots - we can no longer


Typo "replaced".


use a timeout+exit() to avoid slow inputs. Libfuzzer has its own timer
that it uses to catch slow inputs, however these timeouts are usually
seconds-minutes long: more than enough to bog-down the fuzzing process.
However, I found that slow inputs often attempt to fill overly large DMA
requests. Thus, we can mitigate most timeouts by setting a cap on the
total number of DMA bytes written by an input.

Signed-off-by: Alexander Bulekov 
---
  tests/qtest/fuzz/generic_fuzz.c | 5 +
  1 file changed, 5 insertions(+)


Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH 02/10] fuzz: add fuzz_reboot API

2023-02-05 Thread Philippe Mathieu-Daudé

On 5/2/23 05:29, Alexander Bulekov wrote:

As we are converting most fuzzers to rely on reboots to reset state,
introduce an API to make sure reboots are invoked in a consistent
manner.

Signed-off-by: Alexander Bulekov 
---
  tests/qtest/fuzz/fuzz.c | 6 ++
  tests/qtest/fuzz/fuzz.h | 2 +-
  2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index eb7520544b..c2d07a4c7e 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -51,6 +51,12 @@ void flush_events(QTestState *s)
  }
  }
  
+void fuzz_reboot(QTestState *s)


"reboot" sounds like guest software triggered.
IIUC from the fuzzer PoV this is more a "power-cycle" right?


+{
+qemu_system_reset(SHUTDOWN_CAUSE_GUEST_RESET);


Is SHUTDOWN_CAUSE_HOST_QMP_SYSTEM_RESET more appropriate?


+main_loop_wait(true);
+}





Re: [PATCH 2/4] pcie_regs: drop duplicated indicator value macros

2023-02-05 Thread Philippe Mathieu-Daudé

On 4/2/23 18:47, Vladimir Sementsov-Ogievskiy wrote:

We already have indicator values in
include/standard-headers/linux/pci_regs.h , no reason to reinvent them
in include/hw/pci/pcie_regs.h. (and we already have usage of
PCI_EXP_SLTCTL_PWR_IND_BLINK and PCI_EXP_SLTCTL_PWR_IND_OFF in
hw/pci/pcie.c, so let's be consistent)

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  include/hw/pci/pcie_regs.h |  9 -
  hw/pci/pcie.c  | 13 +++--
  2 files changed, 7 insertions(+), 15 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH 1/4] pcie: pcie_cap_slot_write_config(): use correct macro

2023-02-05 Thread Philippe Mathieu-Daudé

On 4/2/23 18:47, Vladimir Sementsov-Ogievskiy wrote:

PCI_EXP_SLTCTL_PIC_OFF is a value, and PCI_EXP_SLTCTL_PIC is a mask.
Happily PCI_EXP_SLTCTL_PIC_OFF is a maximum value for this mask and is
equal to the mask itself. Still the code looks like a bug. Let's make
it more reader-friendly.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  hw/pci/pcie.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH 4/4] pcie: add trace-point for power indicator transitions

2023-02-05 Thread Philippe Mathieu-Daudé

On 4/2/23 18:47, Vladimir Sementsov-Ogievskiy wrote:

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  hw/pci/pcie.c   | 20 
  hw/pci/trace-events |  3 +++
  2 files changed, 23 insertions(+)



+static const char *pcie_sltctl_pic_str(uint16_t sltctl)
+{
+switch (sltctl & PCI_EXP_SLTCTL_PIC) {
+case PCI_EXP_SLTCTL_PWR_IND_ON:
+return "on";
+case PCI_EXP_SLTCTL_PWR_IND_BLINK:
+return "blink";
+case PCI_EXP_SLTCTL_PWR_IND_OFF:
+return "off";
+default:
+return "?";


Maybe "illegal"?

Otherwise:

Reviewed-by: Philippe Mathieu-Daudé 


+}
+}





Re: [PATCH 3/5] hw/ppc/ppc4xx: Set QDev properties using QDev API

2023-02-05 Thread Daniel Henrique Barboza




On 2/3/23 18:16, Philippe Mathieu-Daudé wrote:

No need to use the low-level QOM API when an object
inherits from QDev. Directly use the QDev API to set
its properties.

All calls use either errp=&error_abort or &error_fatal,
so converting to the QDev API is almost a no-op (QDev
API always uses &error_abort).

Signed-off-by: Philippe Mathieu-Daudé 
---


Reviewed-by: Daniel Henrique Barboza 


  hw/ppc/e500.c  | 3 +--
  hw/ppc/ppc405_boards.c | 6 ++
  hw/ppc/ppc405_uc.c | 6 +++---
  hw/ppc/ppc440_bamboo.c | 3 +--
  hw/ppc/ppc4xx_devs.c   | 2 +-
  hw/ppc/sam460ex.c  | 5 ++---
  6 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 9fa1f8e6cf..083961cef5 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -943,8 +943,7 @@ void ppce500_init(MachineState *machine)
   * Secondary CPU starts in halted state for now. Needs to change
   * when implementing non-kernel boot.
   */
-object_property_set_bool(OBJECT(cs), "start-powered-off", i != 0,
- &error_fatal);
+qdev_prop_set_bit(DEVICE(cs), "start-powered-off", i != 0);
  qdev_realize_and_unref(DEVICE(cs), NULL, &error_fatal);
  
  if (!firstenv) {

diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 4092ebc1ab..67eb9ac139 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -276,10 +276,8 @@ static void ppc405_init(MachineState *machine)
  
  object_initialize_child(OBJECT(machine), "soc", &ppc405->soc,

  TYPE_PPC405_SOC);
-object_property_set_link(OBJECT(&ppc405->soc), "dram",
- OBJECT(machine->ram), &error_abort);
-object_property_set_uint(OBJECT(&ppc405->soc), "sys-clk", ,
- &error_abort);
+qdev_prop_set_link(DEVICE(&ppc405->soc), "dram", OBJECT(machine->ram));
+qdev_prop_set_uint32(DEVICE(&ppc405->soc), "sys-clk", );
  qdev_realize(DEVICE(&ppc405->soc), NULL, &error_fatal);
  
  /* allocate and load BIOS */

diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index c973cfb04e..b7d5cfc548 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1080,7 +1080,7 @@ static void ppc405_soc_realize(DeviceState *dev, Error 
**errp)
   * We use the 440 DDR SDRAM controller which has more regs and features
   * but it's compatible enough for now
   */
-object_property_set_int(OBJECT(&s->sdram), "nbanks", 2, &error_abort);
+qdev_prop_set_uint32(DEVICE(&s->sdram), "nbanks", 2);
  if (!ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(&s->sdram), &s->cpu, errp)) {
  return;
  }
@@ -1147,8 +1147,8 @@ static void ppc405_soc_realize(DeviceState *dev, Error 
**errp)
  }
  
  /* MAL */

-object_property_set_int(OBJECT(&s->mal), "txc-num", 4, &error_abort);
-object_property_set_int(OBJECT(&s->mal), "rxc-num", 2, &error_abort);
+qdev_prop_set_uint8(DEVICE(&s->mal), "txc-num", 4);
+qdev_prop_set_uint8(DEVICE(&s->mal), "rxc-num", 2);
  if (!ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(&s->mal), &s->cpu, errp)) {
  return;
  }
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 81d71adf34..3612471990 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -200,8 +200,7 @@ static void bamboo_init(MachineState *machine)
  
  /* SDRAM controller */

  dev = qdev_new(TYPE_PPC4xx_SDRAM_DDR);
-object_property_set_link(OBJECT(dev), "dram", OBJECT(machine->ram),
- &error_abort);
+qdev_prop_set_link(dev, "dram", OBJECT(machine->ram));
  ppc4xx_dcr_realize(PPC4xx_DCR_DEVICE(dev), cpu, &error_fatal);
  object_unref(OBJECT(dev));
  /* XXX 440EP's ECC interrupts are on UIC1, but we've only created UIC0. */
diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index c1d111465d..1848cf5d3c 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -535,7 +535,7 @@ void ppc4xx_dcr_register(Ppc4xxDcrDeviceState *dev, int 
dcrn, void *opaque,
  bool ppc4xx_dcr_realize(Ppc4xxDcrDeviceState *dev, PowerPCCPU *cpu,
  Error **errp)
  {
-object_property_set_link(OBJECT(dev), "cpu", OBJECT(cpu), &error_abort);
+qdev_prop_set_link(DEVICE(dev), "cpu", OBJECT(cpu));
  return sysbus_realize(SYS_BUS_DEVICE(dev), errp);
  }
  
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c

index cf065aae0e..cb828b6d4d 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -345,13 +345,12 @@ static void sam460ex_init(MachineState *machine)
  exit(1);
  }
  dev = qdev_new(TYPE_PPC4xx_SDRAM_DDR2);
-object_property_set_link(OBJECT(dev), "dram", OBJECT(machine->ram),
- &error_abort);
+qdev_prop_set_link(dev, "dram", OBJECT(machine->ram));
  /*
   * Put all RAM on first bank because board has one slot
   * and firmware only checks that
   */
-object_property_set

Re: [PATCH 4/5] hw/ppc/spapr: Set QDev properties using QDev API

2023-02-05 Thread Daniel Henrique Barboza




On 2/3/23 18:16, Philippe Mathieu-Daudé wrote:

No need to use the low-level QOM API when an object
inherits from QDev. Directly use the QDev API to set
its properties.

All calls use either errp=&error_abort or &error_fatal,
so converting to the QDev API is almost a no-op (QDev
API always uses &error_abort).

Signed-off-by: Philippe Mathieu-Daudé 
---


Reviewed-by: Daniel Henrique Barboza 


  hw/intc/spapr_xive.c | 11 ---
  hw/intc/xics.c   |  4 ++--
  hw/intc/xive.c   |  4 ++--
  hw/ppc/spapr_irq.c   |  8 +++-
  4 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index dc641cc604..213c4cac44 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -310,9 +310,8 @@ static void spapr_xive_realize(DeviceState *dev, Error 
**errp)
  /*
   * Initialize the internal sources, for IPIs and virtual devices.
   */
-object_property_set_int(OBJECT(xsrc), "nr-irqs", xive->nr_irqs,
-&error_fatal);
-object_property_set_link(OBJECT(xsrc), "xive", OBJECT(xive), &error_abort);
+qdev_prop_set_uint32(DEVICE(xsrc), "nr-irqs", xive->nr_irqs);
+qdev_prop_set_link(DEVICE(xsrc), "xive", OBJECT(xive));
  if (!qdev_realize(DEVICE(xsrc), NULL, errp)) {
  return;
  }
@@ -321,10 +320,8 @@ static void spapr_xive_realize(DeviceState *dev, Error 
**errp)
  /*
   * Initialize the END ESB source
   */
-object_property_set_int(OBJECT(end_xsrc), "nr-ends", xive->nr_irqs,
-&error_fatal);
-object_property_set_link(OBJECT(end_xsrc), "xive", OBJECT(xive),
- &error_abort);
+qdev_prop_set_uint32(DEVICE(end_xsrc), "nr-ends", xive->nr_irqs);
+qdev_prop_set_link(DEVICE(end_xsrc), "xive", OBJECT(xive));
  if (!qdev_realize(DEVICE(end_xsrc), NULL, errp)) {
  return;
  }
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index c7f8abd71e..2fd1a15153 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -382,8 +382,8 @@ Object *icp_create(Object *cpu, const char *type, 
XICSFabric *xi, Error **errp)
  obj = object_new(type);
  object_property_add_child(cpu, type, obj);
  object_unref(obj);
-object_property_set_link(obj, ICP_PROP_XICS, OBJECT(xi), &error_abort);
-object_property_set_link(obj, ICP_PROP_CPU, cpu, &error_abort);
+qdev_prop_set_link(DEVICE(obj), ICP_PROP_XICS, OBJECT(xi));
+qdev_prop_set_link(DEVICE(obj), ICP_PROP_CPU, cpu);
  if (!qdev_realize(DEVICE(obj), NULL, errp)) {
  object_unparent(obj);
  obj = NULL;
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index a986b96843..0e34035bc6 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -799,8 +799,8 @@ Object *xive_tctx_create(Object *cpu, XivePresenter *xptr, 
Error **errp)
  obj = object_new(TYPE_XIVE_TCTX);
  object_property_add_child(cpu, TYPE_XIVE_TCTX, obj);
  object_unref(obj);
-object_property_set_link(obj, "cpu", cpu, &error_abort);
-object_property_set_link(obj, "presenter", OBJECT(xptr), &error_abort);
+qdev_prop_set_link(DEVICE(obj), "cpu", cpu);
+qdev_prop_set_link(DEVICE(obj), "presenter", OBJECT(xptr));
  if (!qdev_realize(DEVICE(obj), NULL, errp)) {
  object_unparent(obj);
  return NULL;
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index a0d1e1298e..283769c074 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -313,9 +313,8 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
  obj = object_new(TYPE_ICS_SPAPR);
  
  object_property_add_child(OBJECT(spapr), "ics", obj);

-object_property_set_link(obj, ICS_PROP_XICS, OBJECT(spapr),
- &error_abort);
-object_property_set_int(obj, "nr-irqs", smc->nr_xirqs, &error_abort);
+qdev_prop_set_link(DEVICE(obj), ICS_PROP_XICS, OBJECT(spapr));
+qdev_prop_set_uint32(DEVICE(obj), "nr-irqs", smc->nr_xirqs);
  if (!qdev_realize(DEVICE(obj), NULL, errp)) {
  return;
  }
@@ -335,8 +334,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
   * priority
   */
  qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3);
-object_property_set_link(OBJECT(dev), "xive-fabric", OBJECT(spapr),
- &error_abort);
+qdev_prop_set_link(dev, "xive-fabric", OBJECT(spapr));
  sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
  
  spapr->xive = SPAPR_XIVE(dev);




Re: [PATCH 5/5] hw/ppc/pnv: Set QDev properties using QDev API

2023-02-05 Thread Daniel Henrique Barboza




On 2/3/23 18:16, Philippe Mathieu-Daudé wrote:

No need to use the low-level QOM API when an object
inherits from QDev. Directly use the QDev API to set
its properties.

One call in pnv_psi_power8_realize() propagate the
Error* argument:

   if (!object_property_set_int(OBJECT(ics), "nr-irqs",
PSI_NUM_INTERRUPTS, errp)) {
   return;
   }

TYPE_ICS "nr-irqs" is declared in ics_properties[],
itself always registered in ics_class_init(); so converting
this errp to &error_abort is safe.

All other calls use either errp=&error_abort or &error_fatal,
so converting to the QDev API is almost a no-op (QDev API
always uses &error_abort).

Signed-off-by: Philippe Mathieu-Daudé 
---


Reviewed-by: Daniel Henrique Barboza 


  hw/intc/pnv_xive.c | 11 --
  hw/intc/pnv_xive2.c| 15 +-
  hw/pci-host/pnv_phb3.c |  9 +++--
  hw/pci-host/pnv_phb4.c |  4 ++--
  hw/pci-host/pnv_phb4_pec.c | 10 +++---
  hw/ppc/pnv.c   | 41 --
  hw/ppc/pnv_psi.c   | 10 +++---
  7 files changed, 37 insertions(+), 63 deletions(-)

diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
index 622f9d28b7..ccc1ea5380 100644
--- a/hw/intc/pnv_xive.c
+++ b/hw/intc/pnv_xive.c
@@ -1857,17 +1857,14 @@ static void pnv_xive_realize(DeviceState *dev, Error 
**errp)
   * resized dynamically when the controller is configured by the FW
   * to limit accesses to resources not provisioned.
   */
-object_property_set_int(OBJECT(xsrc), "nr-irqs", PNV_XIVE_NR_IRQS,
-&error_fatal);
-object_property_set_link(OBJECT(xsrc), "xive", OBJECT(xive), &error_abort);
+qdev_prop_set_uint32(DEVICE(xsrc), "nr-irqs", PNV_XIVE_NR_IRQS);
+qdev_prop_set_link(DEVICE(xsrc), "xive", OBJECT(xive));
  if (!qdev_realize(DEVICE(xsrc), NULL, errp)) {
  return;
  }
  
-object_property_set_int(OBJECT(end_xsrc), "nr-ends", PNV_XIVE_NR_ENDS,

-&error_fatal);
-object_property_set_link(OBJECT(end_xsrc), "xive", OBJECT(xive),
- &error_abort);
+qdev_prop_set_uint32(DEVICE(end_xsrc), "nr-ends", PNV_XIVE_NR_ENDS);
+qdev_prop_set_link(DEVICE(end_xsrc), "xive", OBJECT(xive));
  if (!qdev_realize(DEVICE(end_xsrc), NULL, errp)) {
  return;
  }
diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c
index 7176d70234..d7695f65e7 100644
--- a/hw/intc/pnv_xive2.c
+++ b/hw/intc/pnv_xive2.c
@@ -1821,22 +1821,17 @@ static void pnv_xive2_realize(DeviceState *dev, Error 
**errp)
   * resized dynamically when the controller is configured by the FW
   * to limit accesses to resources not provisioned.
   */
-object_property_set_int(OBJECT(xsrc), "flags", XIVE_SRC_STORE_EOI,
-&error_fatal);
-object_property_set_int(OBJECT(xsrc), "nr-irqs", PNV_XIVE2_NR_IRQS,
-&error_fatal);
-object_property_set_link(OBJECT(xsrc), "xive", OBJECT(xive),
- &error_fatal);
+qdev_prop_set_uint64(DEVICE(xsrc), "flags", XIVE_SRC_STORE_EOI);
+qdev_prop_set_uint32(DEVICE(xsrc), "nr-irqs", PNV_XIVE2_NR_IRQS);
+qdev_prop_set_link(DEVICE(xsrc), "xive", OBJECT(xive));
  qdev_realize(DEVICE(xsrc), NULL, &local_err);
  if (local_err) {
  error_propagate(errp, local_err);
  return;
  }
  
-object_property_set_int(OBJECT(end_xsrc), "nr-ends", PNV_XIVE2_NR_ENDS,

-&error_fatal);
-object_property_set_link(OBJECT(end_xsrc), "xive", OBJECT(xive),
- &error_abort);
+qdev_prop_set_uint32(DEVICE(end_xsrc), "nr-ends", PNV_XIVE2_NR_ENDS);
+qdev_prop_set_link(DEVICE(end_xsrc), "xive", OBJECT(xive));
  qdev_realize(DEVICE(end_xsrc), NULL, &local_err);
  if (local_err) {
  error_propagate(errp, local_err);
diff --git a/hw/pci-host/pnv_phb3.c b/hw/pci-host/pnv_phb3.c
index 7a21497cf8..6da9053ffa 100644
--- a/hw/pci-host/pnv_phb3.c
+++ b/hw/pci-host/pnv_phb3.c
@@ -1029,8 +1029,7 @@ static void pnv_phb3_realize(DeviceState *dev, Error 
**errp)
  /* LSI sources */
  object_property_set_link(OBJECT(&phb->lsis), "xics", OBJECT(pnv),
   &error_abort);
-object_property_set_int(OBJECT(&phb->lsis), "nr-irqs", PNV_PHB3_NUM_LSI,
-&error_abort);
+qdev_prop_set_uint32(DEVICE(&phb->lsis), "nr-irqs", PNV_PHB3_NUM_LSI);
  if (!qdev_realize(DEVICE(&phb->lsis), NULL, errp)) {
  return;
  }
@@ -1046,15 +1045,13 @@ static void pnv_phb3_realize(DeviceState *dev, Error 
**errp)
   &error_abort);
  object_property_set_link(OBJECT(&phb->msis), "xics", OBJECT(pnv),
   &error_abort);
-object_property_set_int(OBJECT(&phb->msis), "nr-irqs", PHB3_MAX_MSI,
-&error_abort);
+qdev_prop

Re: [PATCH 2/5] hw/pci-host/raven: Set QDev properties using QDev API

2023-02-05 Thread Daniel Henrique Barboza




On 2/3/23 18:16, Philippe Mathieu-Daudé wrote:

No need to use the low-level QOM API when an object
inherits from QDev. Directly use the QDev API to set
its properties.

All calls use either errp=&error_fatal or NULL, so
converting to the QDev API is almost a no-op (QDev API
always uses &error_abort).

Signed-off-by: Philippe Mathieu-Daudé 
---


Reviewed-by: Daniel Henrique Barboza 


  hw/pci-host/raven.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
index cdfb62ac2e..2c842d2146 100644
--- a/hw/pci-host/raven.c
+++ b/hw/pci-host/raven.c
@@ -246,8 +246,7 @@ static void raven_pcihost_realizefn(DeviceState *d, Error 
**errp)
  /* According to PReP specification section 6.1.6 "System Interrupt
   * Assignments", all PCI interrupts are routed via IRQ 15 */
  s->or_irq = OR_IRQ(object_new(TYPE_OR_IRQ));
-object_property_set_int(OBJECT(s->or_irq), "num-lines", PCI_NUM_PINS,
-&error_fatal);
+qdev_prop_set_uint16(DEVICE(s->or_irq), "num-lines", PCI_NUM_PINS);
  qdev_realize(DEVICE(s->or_irq), NULL, &error_fatal);
  sysbus_init_irq(dev, &s->or_irq->out_irq);
  
@@ -319,8 +318,7 @@ static void raven_pcihost_initfn(Object *obj)
  
  object_initialize(&s->pci_dev, sizeof(s->pci_dev), TYPE_RAVEN_PCI_DEVICE);

  pci_dev = DEVICE(&s->pci_dev);
-object_property_set_int(OBJECT(&s->pci_dev), "addr", PCI_DEVFN(0, 0),
-NULL);
+qdev_prop_set_int32(pci_dev, "addr", PCI_DEVFN(0, 0));
  qdev_prop_set_bit(pci_dev, "multifunction", false);
  }
  




Re: [PATCH v3 1/9] hw/pci-host/i440fx: Inline sysbus_add_io()

2023-02-05 Thread Philippe Mathieu-Daudé

On 4/2/23 16:10, Bernhard Beschow wrote:

sysbus_add_io() just wraps memory_region_add_subregion() while also
obscuring where the memory is attached. So use
memory_region_add_subregion() directly and attach it to the existing
memory region s->bus->address_space_io which is set as an alias to
get_system_io() by the pc machine.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Thomas Huth 
---
  hw/pci-host/i440fx.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 262f82c303..9c6882d3fc 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -27,6 +27,7 @@
  #include "qemu/range.h"
  #include "hw/i386/pc.h"
  #include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
  #include "hw/pci/pci_host.h"
  #include "hw/pci-host/i440fx.h"
  #include "hw/qdev-properties.h"
@@ -217,10 +218,10 @@ static void i440fx_pcihost_realize(DeviceState *dev, 
Error **errp)
  PCIHostState *s = PCI_HOST_BRIDGE(dev);
  SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
  
-sysbus_add_io(sbd, 0xcf8, &s->conf_mem);

+memory_region_add_subregion(s->bus->address_space_io, 0xcf8, &s->conf_mem);


address_space_io is internal to PCIBus. Better would be adding a helper
like pci_bus_io_region() IMO. Anyhow, removing sysbus_add_io() uses is
good, so:

Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH 0/5] hw/ppc: Set QDev properties using QDev API (part 2/3)

2023-02-05 Thread Daniel Henrique Barboza




On 2/3/23 18:16, Philippe Mathieu-Daudé wrote:

part 1 [*] cover:
--
QEMU provides the QOM API for core objects.
Devices are modelled on top of QOM as QDev objects.

There is no point in using the lower level QOM API with
QDev; it makes the code more complex and harder to review.

I first converted all the calls using errp=&error_abort or
&errp=NULL, then noticed the other uses weren't really
consistent.

A QDev property defined with the DEFINE_PROP_xxx() macros
is always available, thus can't fail. When using hot-plug
devices, we only need to check for optional properties
registered at runtime with the object_property_add_XXX()
API. Some are even always registered in device instance_init.
--

In this series PPC hw is converted. Only one call site in PNV
forwards the Error* argument and its conversion is justified.



Feel free to take the 4 patches I acked via your tree when pushing it together
with part 1/3.

I can't ack macio because that's Mark's turf.

Thanks,


Daniel



Based-on: <20230203180914.49112-1-phi...@linaro.org>
(in particular [PATCH 02/19] hw/qdev: Introduce qdev_prop_set_link():
  https://lore.kernel.org/qemu-devel/20230203180914.49112-3-phi...@linaro.org/)

[*] https://lore.kernel.org/qemu-devel/20230203180914.49112-1-phi...@linaro.org/

Philippe Mathieu-Daudé (5):
   hw/misc/macio: Set QDev properties using QDev API
   hw/pci-host/raven: Set QDev properties using QDev API
   hw/ppc/ppc4xx: Set QDev properties using QDev API
   hw/ppc/spapr: Set QDev properties using QDev API
   hw/ppc/pnv: Set QDev properties using QDev API

  hw/intc/pnv_xive.c | 11 --
  hw/intc/pnv_xive2.c| 15 +-
  hw/intc/spapr_xive.c   | 11 --
  hw/intc/xics.c |  4 ++--
  hw/intc/xive.c |  4 ++--
  hw/misc/macio/macio.c  |  6 ++
  hw/pci-host/pnv_phb3.c |  9 +++--
  hw/pci-host/pnv_phb4.c |  4 ++--
  hw/pci-host/pnv_phb4_pec.c | 10 +++---
  hw/pci-host/raven.c|  6 ++
  hw/ppc/e500.c  |  3 +--
  hw/ppc/pnv.c   | 41 --
  hw/ppc/pnv_psi.c   | 10 +++---
  hw/ppc/ppc405_boards.c |  6 ++
  hw/ppc/ppc405_uc.c |  6 +++---
  hw/ppc/ppc440_bamboo.c |  3 +--
  hw/ppc/ppc4xx_devs.c   |  2 +-
  hw/ppc/sam460ex.c  |  5 ++---
  hw/ppc/spapr_irq.c |  8 +++-
  19 files changed, 62 insertions(+), 102 deletions(-)





Re: [PATCH v3 2/9] hw/pci-host/q35: Inline sysbus_add_io()

2023-02-05 Thread Philippe Mathieu-Daudé

On 4/2/23 16:10, Bernhard Beschow wrote:

sysbus_add_io() just wraps memory_region_add_subregion() while also
obscuring where the memory is attached. So use
memory_region_add_subregion() directly and attach it to the existing
memory region s->mch.address_space_io which is set as an alias to
get_system_io() by the q35 machine.

Signed-off-by: Bernhard Beschow 
Reviewed-by: Thomas Huth 
---
  hw/pci-host/q35.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 26390863d6..fa05844319 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -50,10 +50,12 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
  Q35PCIHost *s = Q35_HOST_DEVICE(dev);
  SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
  
-sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);

+memory_region_add_subregion(s->mch.address_space_io,
+MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
  sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);


This makes me wonder why MCH_PCI_DEVICE doesn't use the bus I/O space
via pci_address_space_io(). IOW, why the MR like is in MCH_PCI_DEVICE
and not Q35_HOST_DEVICE?



Re: [PATCH v3 3/9] hw/i386/pc_q35: Reuse machine parameter

2023-02-05 Thread Philippe Mathieu-Daudé

On 4/2/23 16:10, Bernhard Beschow wrote:

Signed-off-by: Bernhard Beschow 
Reviewed-by: Thomas Huth 
---
  hw/i386/pc_q35.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH v3 5/9] hw/i386/pc_{q35, piix}: Minimize usage of get_system_memory()

2023-02-05 Thread Philippe Mathieu-Daudé

On 4/2/23 16:10, Bernhard Beschow wrote:

Signed-off-by: Bernhard Beschow 
Reviewed-by: Thomas Huth 
---
  hw/i386/pc_piix.c | 2 +-
  hw/i386/pc_q35.c  | 7 ---
  2 files changed, 5 insertions(+), 4 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH v3 7/9] hw/pci-host/pam: Make init_pam() usage more readable

2023-02-05 Thread Philippe Mathieu-Daudé

On 4/2/23 16:10, Bernhard Beschow wrote:

Unlike pam_update() which takes the subject -- PAMMemoryRegion -- as
first argument, init_pam() takes it as fifth (!) argument. This makes it
quite hard to figure out what an init_pam() invocation actually
initializes. By moving the subject to the front this should become
clearer.

While at it, lower the DeviceState parameter to Object, also
communicating more clearly that this parameter is just the owner rather
than some (heavy?) dependency.

Signed-off-by: Bernhard Beschow 
---
  include/hw/pci-host/pam.h |  5 +++--
  hw/pci-host/i440fx.c  | 10 +-
  hw/pci-host/pam.c | 12 ++--
  hw/pci-host/q35.c |  8 
  4 files changed, 18 insertions(+), 17 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH v3 8/9] hw/i386/x86: Make TYPE_X86_MACHINE the owner of smram

2023-02-05 Thread Philippe Mathieu-Daudé

On 4/2/23 16:10, Bernhard Beschow wrote:

Treat the smram MemoryRegion analoguous to other memory regions such as
ram, pci, io, ... , making the used memory regions more explicit when
instantiating q35 or i440fx.

Note that the q35 device uses these memory regions only during the
realize phase which suggests that it shouldn't be the owner of smram.


Few years ago I tried something similar and it wasn't accepted because
the MR owner name is used in the migration stream, so this was breaking
migrating from older machines.

Adding David/Juan for double-checking that.


i440fx activates/deactivates the whole smram memory region depending on
the SMRAM_G_SMRAME bit which seems wrong since it should only handle the
128kb region. If this got changed, i440fx would also only use smram
during its realize phase.

Signed-off-by: Bernhard Beschow 
---
  include/hw/i386/x86.h|  2 ++
  include/hw/pci-host/i440fx.h |  7 ---
  include/hw/pci-host/q35.h|  4 +++-
  hw/i386/pc_piix.c|  2 +-
  hw/i386/pc_q35.c |  2 ++
  hw/i386/x86.c|  4 
  hw/pci-host/i440fx.c | 13 +
  hw/pci-host/q35.c| 17 -
  8 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index 62fa5774f8..ba6912b721 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -59,6 +59,8 @@ struct X86MachineState {
  /* Start address of the initial RAM above 4G */
  uint64_t above_4g_mem_start;
  
+MemoryRegion smram;

+
  /* CPU and apic information: */
  bool apic_xrupt_override;
  unsigned pci_irq_mask;
diff --git a/include/hw/pci-host/i440fx.h b/include/hw/pci-host/i440fx.h
index bf57216c78..e9efdb3c5f 100644
--- a/include/hw/pci-host/i440fx.h
+++ b/include/hw/pci-host/i440fx.h
@@ -28,9 +28,10 @@ struct PCII440FXState {
  MemoryRegion *system_memory;
  MemoryRegion *pci_address_space;
  MemoryRegion *ram_memory;
+MemoryRegion *smram;
  PAMMemoryRegion pam_regions[PAM_REGIONS_COUNT];
  MemoryRegion smram_region;
-MemoryRegion smram, low_smram;
+MemoryRegion low_smram;
  };
  
  #define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "igd-passthrough-i440FX"

@@ -43,7 +44,7 @@ PCIBus *i440fx_init(const char *pci_type,
  ram_addr_t below_4g_mem_size,
  ram_addr_t above_4g_mem_size,
  MemoryRegion *pci_memory,
-MemoryRegion *ram_memory);
-
+MemoryRegion *ram_memory,
+MemoryRegion *smram);
  
  #endif

diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index e89329c51e..fcbe57b42d 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -44,9 +44,10 @@ struct MCHPCIState {
  MemoryRegion *pci_address_space;
  MemoryRegion *system_memory;
  MemoryRegion *address_space_io;
+MemoryRegion *smram;
  PAMMemoryRegion pam_regions[PAM_REGIONS_COUNT];
  MemoryRegion smram_region, open_high_smram;
-MemoryRegion smram, low_smram, high_smram;
+MemoryRegion low_smram, high_smram;
  MemoryRegion tseg_blackhole, tseg_window;
  MemoryRegion smbase_blackhole, smbase_window;
  bool has_smram_at_smbase;
@@ -75,6 +76,7 @@ struct Q35PCIHost {
   */
  
  #define MCH_HOST_PROP_RAM_MEM "ram-mem"

+#define MCH_HOST_PROP_SMRAM_MEM "smram-mem"
  #define MCH_HOST_PROP_PCI_MEM "pci-mem"
  #define MCH_HOST_PROP_SYSTEM_MEM "system-mem"
  #define MCH_HOST_PROP_IO_MEM "io-mem"
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 00ba725656..415465 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -228,7 +228,7 @@ static void pc_init1(MachineState *machine,
system_memory, system_io, machine->ram_size,
x86ms->below_4g_mem_size,
x86ms->above_4g_mem_size,
-  pci_memory, ram_memory);
+  pci_memory, ram_memory, &x86ms->smram);
  pci_bus_map_irqs(pci_bus,
   xen_enabled() ? xen_pci_slot_get_pirq
 : pc_pci_slot_get_pirq);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 88f0981f50..480226de2c 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -221,6 +221,8 @@ static void pc_q35_init(MachineState *machine)
  object_property_add_child(OBJECT(machine), "q35", OBJECT(q35_host));
  object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
   OBJECT(ram_memory), NULL);
+object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_SMRAM_MEM,
+ OBJECT(&x86ms->smram), NULL);
  object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_PCI_MEM,
   OBJECT(pci_memory), NULL);
  object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_SYSTEM_MEM,
diff --git a/hw/i

Re: [PATCH v3 9/9] target/i386/tcg/sysemu/tcg-cpu: Avoid own opinion about smram size

2023-02-05 Thread Philippe Mathieu-Daudé

On 4/2/23 16:10, Bernhard Beschow wrote:

When setting up the CPU's smram memory region alias, the code currently
assumes that the smram size is 4 GiB. While this is true, it repeats a
decision made elsewhere which seems redundant and prone to
inconsistencies. Avoid this by reusing whatever size the smram region
was set to.

Signed-off-by: Bernhard Beschow 
---
  target/i386/tcg/sysemu/tcg-cpu.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/i386/tcg/sysemu/tcg-cpu.c b/target/i386/tcg/sysemu/tcg-cpu.c
index c223c0fe9b..8f5ce6093c 100644
--- a/target/i386/tcg/sysemu/tcg-cpu.c
+++ b/target/i386/tcg/sysemu/tcg-cpu.c
@@ -22,7 +22,6 @@
  #include "tcg/helper-tcg.h"
  
  #include "sysemu/sysemu.h"

-#include "qemu/units.h"
  #include "exec/address-spaces.h"
  
  #include "tcg/tcg-cpu.h"

@@ -36,7 +35,7 @@ static void tcg_cpu_machine_done(Notifier *n, void *unused)
  if (smram) {
  cpu->smram = g_new(MemoryRegion, 1);
  memory_region_init_alias(cpu->smram, OBJECT(cpu), "smram",
- smram, 0, 4 * GiB);
+ smram, 0, memory_region_size(smram));


Or define SMRAM_REGION_SIZE and use it?

(subject header can be shortened to "target/i386:").



Re: [PATCH 1/3] docs/system: Remove "mips" board from target-mips.rst

2023-02-05 Thread Philippe Mathieu-Daudé

On 2/2/23 14:21, Jiaxun Yang wrote:

This board had been deprecated long ago.


s/deprecated/removed/

Indeed, in commit f169413c27 ("hw/mips: Remove the 'r4k' machine").



Signed-off-by: Jiaxun Yang 
---
  docs/system/target-mips.rst | 14 --
  1 file changed, 14 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH 3/3] hw/mips: Add MIPS virt board

2023-02-05 Thread Philippe Mathieu-Daudé

Hi Jiaxun,

On 2/2/23 14:21, Jiaxun Yang wrote:

MIPS virt board is design to utilize existing VirtIO infrastures
but also comptitable with MIPS's existing internal simulation tools.

It includes virtio-mmio, pcie gpex, flash rom, fw_cfg, goldfish-rtc,
and optional goldfish_pic in case MIPS GIC is not present.


Is it worth using the CPS/GIC? Can't we using the goldfish PIC
regardless CPS availability? Did you run performance comparison?



It should be able to cooperate with any MIPS CPU cores.

Signed-off-by: Jiaxun Yang 
---
v1:
  - Rename to virt board
  - Convert BIOS flash to ROM
  - Cleanups
---
  MAINTAINERS |7 +
  configs/devices/mips-softmmu/common.mak |1 +
  docs/system/target-mips.rst |   24 +
  hw/mips/Kconfig |   18 +
  hw/mips/meson.build |1 +
  hw/mips/virt.c  | 1015 +++
  6 files changed, 1066 insertions(+)
  create mode 100644 hw/mips/virt.c





Re: [PULL 00/11] Net patches

2023-02-05 Thread Peter Maydell
On Sat, 4 Feb 2023 at 20:09, Laurent Vivier  wrote:
>
> On 2/4/23 15:57, Peter Maydell wrote:
> > On Thu, 2 Feb 2023 at 06:21, Jason Wang  wrote:
> >>
> >> The following changes since commit 
> >> 13356edb87506c148b163b8c7eb0695647d00c2a:
> >>
> >>Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into 
> >> staging (2023-01-24 09:45:33 +)
> >>
> >> are available in the git repository at:
> >>
> >>https://github.com/jasowang/qemu.git tags/net-pull-request
> >>
> >> for you to fetch changes up to 2bd492bca521ee8594f1d5db8dc9aac126fc4f85:
> >>
> >>vdpa: fix VHOST_BACKEND_F_IOTLB_ASID flag check (2023-02-02 14:16:48 
> >> +0800)
> >>
> >> 
> >
> > Something weird has happened here -- this pullreq is trying to
> > add tests/qtest/netdev-socket.c, but it already exists in the
> > tree and doesn't have the same contents as the version in your
> > pull request.
> >
> > Can you look at what's happened here and fix it up, please ?
>
> Thomas and Jason have queued the patch:
>
>tests/qtest: netdev: test stream and dgram backends
>
> For Jason it's because it's needed by
>
>net: stream: add a new option to automatically reconnect
>
> For me, both patches (in tree and Jason's one) are identical to my v7
> (except the one that is merged does not have Thomas' acked-by).

When I tried to merge the pullreq I got conflicts because they
weren't the same, notably in the set of #include lines.

thanks
-- PMM



Re: Call for GSoC and Outreachy project ideas for summer 2023

2023-02-05 Thread Stefan Hajnoczi
On Sun, 5 Feb 2023 at 03:15, Eugenio Perez Martin  wrote:
>
> On Fri, Jan 27, 2023 at 4:18 PM Stefan Hajnoczi  wrote:
> >
> > Dear QEMU, KVM, and rust-vmm communities,
> > QEMU will apply for Google Summer of Code 2023
> > (https://summerofcode.withgoogle.com/) and has been accepted into
> > Outreachy May 2023 (https://www.outreachy.org/). You can now
> > submit internship project ideas for QEMU, KVM, and rust-vmm!
> >
> > Please reply to this email by February 6th with your project ideas.
> >
> > If you have experience contributing to QEMU, KVM, or rust-vmm you can
> > be a mentor. Mentors support interns as they work on their project. It's a
> > great way to give back and you get to work with people who are just
> > starting out in open source.
> >
> > Good project ideas are suitable for remote work by a competent
> > programmer who is not yet familiar with the codebase. In
> > addition, they are:
> > - Well-defined - the scope is clear
> > - Self-contained - there are few dependencies
> > - Uncontroversial - they are acceptable to the community
> > - Incremental - they produce deliverables along the way
> >
> > Feel free to post ideas even if you are unable to mentor the project.
> > It doesn't hurt to share the idea!
> >
> > I will review project ideas and keep you up-to-date on QEMU's
> > acceptance into GSoC.
> >
> > Internship program details:
> > - Paid, remote work open source internships
> > - GSoC projects are 175 or 350 hours, Outreachy projects are 30
> > hrs/week for 12 weeks
> > - Mentored by volunteers from QEMU, KVM, and rust-vmm
> > - Mentors typically spend at least 5 hours per week during the coding period
> >
> > For more background on QEMU internships, check out this video:
> > https://www.youtube.com/watch?v=xNVCX7YMUL8
> >
> > Please let me know if you have any questions!
> >
> > Stefan
> >
>
> Appending the different ideas here.

Hi Eugenio,
Thanks for sharing your project ideas. I have added some questions
below before we add them to the ideas list wiki page.

> VIRTIO_F_IN_ORDER feature support for virtio devices
> ===
> This was already a project the last year, and it produced a few series
> upstream but was never merged. The previous series are totally useful
> to start with, so it's not starting from scratch with them [1]:

Has Zhi Guo stopped working on the patches?

What is the state of the existing patches? What work remains to be done?

>
> Summary
> ---
> Implement VIRTIO_F_IN_ORDER in QEMU and Linux (vhost and virtio drivers)
>
> The VIRTIO specification defines a feature bit (VIRTIO_F_IN_ORDER)
> that devices and drivers can negotiate when the device uses
> descriptors in the same order in which they were made available by the
> driver.
>
> This feature can simplify device and driver implementations and
> increase performance. For example, when VIRTIO_F_IN_ORDER is
> negotiated, it may be easier to create a batch of buffers and reduce
> DMA transactions when the device uses a batch of buffers.
>
> Currently the devices and drivers available in Linux and QEMU do not
> support this feature. An implementation is available in DPDK for the
> virtio-net driver.
>
> Goals
> ---
> Implement VIRTIO_F_IN_ORDER for a single device/driver in QEMU and
> Linux (virtio-net or virtio-serial are good starting points).
> Generalize your approach to the common virtio core code for split and
> packed virtqueue layouts.
> If time allows, support for the packed virtqueue layout can be added
> to Linux vhost, QEMU's libvhost-user, and/or QEMU's virtio qtest code.
>
> Shadow Virtqueue missing virtio features
> ===
>
> Summary
> ---
> Some VirtIO devices like virtio-net have a control virtqueue (CVQ)
> that allows them to dynamically change a number of parameters like MAC
> or number of active queues. Changes to passthrough devices using vDPA
> using CVQ are inherently hard to track if CVQ is handled as
> passthrough data queues, because qemu is not aware of that
> communication for performance reasons. In this situation, qemu is not
> able to migrate these devices, as it is not able to tell the actual
> state of the device.
>
> Shadow Virtqueue (SVQ) allows qemu to offer an emulated queue to the
> device, effectively forwarding the descriptors of that communication,
> tracking the device internal state, and being able to migrate it to a
> new destination qemu.
>
> To restore that state in the destination, SVQ is able to send these
> messages as regular CVQ commands. The code to understand and parse
> virtio-net CVQ commands is already in qemu as part of its emulated
> device, but the code to send the some of the new state is not, and
> some features are missing. There is already code to restore basic
> commands like mac or multiqueue, and it is easy to use it as a
> template.
>
> Goals
> ---
> To implement missing virtio-net commands sending:
> * VIRTIO_NET_CTRL_RX family, to control receive mode.
> * VIRTIO_NET_CTRL_GUEST_OFFLOADS
> * VIRTIO_NET_CTRL_VLAN family
> * VIRTIO_NET_CTRL_MQ_HASH co

Qemu - how to run in Win?

2023-02-05 Thread Jacob A
Hello,

After installing Qemu on Win, I don't see any shortcut to run it? There is
only a link to 'uninstall'. launching exe files doesn't do anything.  Can
you please explain how to launch this application?

Thanks,
J.

Please see the attached image.


[PATCH v2 2/2] tcg: use QTree instead of GTree

2023-02-05 Thread Emilio Cota
qemu-user can hang in a multi-threaded fork. One common
reason is that when creating a TB, between fork and exec
we manipulate a GTree whose memory allocator (GSlice) is
not fork-safe.

Although POSIX does not mandate it, the system's allocator
(e.g. tcmalloc, libc malloc) is probably fork-safe.

Fix some of these hangs by using QTree, which uses the system's
allocator regardless of the Glib version that we used at
configuration time.

Tested with the test program in the original bug report, i.e.:
```

void garble() {
  int pid = fork();
  if (pid == 0) {
exit(0);
  } else {
int wstatus;
waitpid(pid, &wstatus, 0);
  }
}

void supragarble(unsigned depth) {
  if (depth == 0)
return ;

  std::thread a(supragarble, depth-1);
  std::thread b(supragarble, depth-1);
  garble();
  a.join();
  b.join();
}

int main() {
  supragarble(10);
}
```

Fixes: #285

Signed-off-by: Emilio Cota 
---
 accel/tcg/tb-maint.c | 17 +
 tcg/region.c | 19 ++-
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/accel/tcg/tb-maint.c b/accel/tcg/tb-maint.c
index b3d6529ae2..e6370ddc71 100644
--- a/accel/tcg/tb-maint.c
+++ b/accel/tcg/tb-maint.c
@@ -19,6 +19,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/interval-tree.h"
+#include "qemu/qtree.h"
 #include "exec/cputlb.h"
 #include "exec/log.h"
 #include "exec/exec-all.h"
@@ -313,7 +314,7 @@ struct page_entry {
  * See also: page_collection_lock().
  */
 struct page_collection {
-GTree *tree;
+QTree *tree;
 struct page_entry *max;
 };
 
@@ -466,7 +467,7 @@ static bool page_trylock_add(struct page_collection *set, 
tb_page_addr_t addr)
 struct page_entry *pe;
 PageDesc *pd;
 
-pe = g_tree_lookup(set->tree, &index);
+pe = q_tree_lookup(set->tree, &index);
 if (pe) {
 return false;
 }
@@ -477,7 +478,7 @@ static bool page_trylock_add(struct page_collection *set, 
tb_page_addr_t addr)
 }
 
 pe = page_entry_new(pd, index);
-g_tree_insert(set->tree, &pe->index, pe);
+q_tree_insert(set->tree, &pe->index, pe);
 
 /*
  * If this is either (1) the first insertion or (2) a page whose index
@@ -524,13 +525,13 @@ static struct page_collection 
*page_collection_lock(tb_page_addr_t start,
 end   >>= TARGET_PAGE_BITS;
 g_assert(start <= end);
 
-set->tree = g_tree_new_full(tb_page_addr_cmp, NULL, NULL,
+set->tree = q_tree_new_full(tb_page_addr_cmp, NULL, NULL,
 page_entry_destroy);
 set->max = NULL;
 assert_no_pages_locked();
 
  retry:
-g_tree_foreach(set->tree, page_entry_lock, NULL);
+q_tree_foreach(set->tree, page_entry_lock, NULL);
 
 for (index = start; index <= end; index++) {
 TranslationBlock *tb;
@@ -541,7 +542,7 @@ static struct page_collection 
*page_collection_lock(tb_page_addr_t start,
 continue;
 }
 if (page_trylock_add(set, index << TARGET_PAGE_BITS)) {
-g_tree_foreach(set->tree, page_entry_unlock, NULL);
+q_tree_foreach(set->tree, page_entry_unlock, NULL);
 goto retry;
 }
 assert_page_locked(pd);
@@ -550,7 +551,7 @@ static struct page_collection 
*page_collection_lock(tb_page_addr_t start,
 (tb_page_addr1(tb) != -1 &&
  page_trylock_add(set, tb_page_addr1(tb {
 /* drop all locks, and reacquire in order */
-g_tree_foreach(set->tree, page_entry_unlock, NULL);
+q_tree_foreach(set->tree, page_entry_unlock, NULL);
 goto retry;
 }
 }
@@ -561,7 +562,7 @@ static struct page_collection 
*page_collection_lock(tb_page_addr_t start,
 static void page_collection_unlock(struct page_collection *set)
 {
 /* entries are unlocked and freed via page_entry_destroy */
-g_tree_destroy(set->tree);
+q_tree_destroy(set->tree);
 g_free(set);
 }
 
diff --git a/tcg/region.c b/tcg/region.c
index 88d6bb273f..bef4c4756f 100644
--- a/tcg/region.c
+++ b/tcg/region.c
@@ -28,6 +28,7 @@
 #include "qemu/mprotect.h"
 #include "qemu/memalign.h"
 #include "qemu/cacheinfo.h"
+#include "qemu/qtree.h"
 #include "qapi/error.h"
 #include "exec/exec-all.h"
 #include "tcg/tcg.h"
@@ -36,7 +37,7 @@
 
 struct tcg_region_tree {
 QemuMutex lock;
-GTree *tree;
+QTree *tree;
 /* padding to avoid false sharing is computed at run-time */
 };
 
@@ -163,7 +164,7 @@ static void tcg_region_trees_init(void)
 struct tcg_region_tree *rt = region_trees + i * tree_size;
 
 qemu_mutex_init(&rt->lock);
-rt->tree = g_tree_new_full(tb_tc_cmp, NULL, NULL, tb_destroy);
+rt->tree = q_tree_new_full(tb_tc_cmp, NULL, NULL, tb_destroy);
 }
 }
 
@@ -202,7 +203,7 @@ void tcg_tb_insert(TranslationBlock *tb)
 
 g_assert(rt != NULL);
 qemu_mutex_lock(&rt->lock);
-g_tree_insert(rt->tree, &tb->tc, tb);
+q_tree_insert(rt->tree, &tb->tc, tb);
 qemu_mutex_unlock(&rt->lock);
 }
 
@

[PATCH v2 0/2] fix for #285

2023-02-05 Thread Emilio Cota
Changes since v1:

- Add configure check to only use QTree if Glib still implements gslice.
  If Glib doesn't, then we call Glib directly with inline functions.
- Add TODO's so that in the future (i.e. when the minimum version of
  Glib that we use doesn't implement gslice) we remove QTree.
- Add comment to the top of qtree.h.
- Make qtree-bench results more robust by running longer or more times.
- Drop deprecated API calls (they're unused in QEMU).
- Drop API calls that are too recent (they're unused in QEMU).
- Drop macro benchmark results from the TCG patch since they're too noisy.
- Add test program to the commit log so that we don't lose it in the future
  even if the bug tracker goes away.

Thanks,
Emilio





[PATCH v2 1/2] util: import GTree as QTree

2023-02-05 Thread Emilio Cota
The only reason to add this implementation is to control the memory allocator
used. Some users (e.g. TCG) cannot work reliably in multi-threaded
environments (e.g. forking in user-mode) with GTree's allocator, GSlice.
See https://gitlab.com/qemu-project/qemu/-/issues/285 for details.

Importing GTree is a temporary workaround until GTree migrates away
from GSlice.

This implementation is identical to that in glib v2.75.0, except that
we don't import recent additions to the API nor deprecated API calls,
none of which are used in QEMU.

I've imported tests from glib and added a benchmark just to
make sure that performance is similar. Note: it cannot be identical
because (1) we are not using GSlice, (2) we use different compilation flags
(e.g. -fPIC) and (3) we're linking statically.

$ cat /proc/cpuinfo| grep 'model name' | head -1
model name  : AMD Ryzen 7 PRO 5850U with Radeon Graphics
$ echo '0' | sudo tee /sys/devices/system/cpu/cpufreq/boost
$ tests/bench/qtree-bench

 Tree Op  3210244096  131072
 1048576

GTree Lookup   83.23   43.08   25.31   19.40
   16.22
QTree Lookup  113.42 (1.36x)   53.83 (1.25x)   28.38 (1.12x)   17.64 
(0.91x)   13.04 (0.80x)
GTree Insert   44.23   29.37   25.83   19.49
   17.03
QTree Insert   46.87 (1.06x)   25.62 (0.87x)   24.29 (0.94x)   16.83 
(0.86x)   12.97 (0.76x)
GTree Remove   53.27   35.15   31.43   24.64
   16.70
QTree Remove   57.32 (1.08x)   41.76 (1.19x)   38.37 (1.22x)   29.30 
(1.19x)   15.07 (0.90x)
GTree  RemoveAll  135.44  127.52  126.72  120.11
   64.34
QTree  RemoveAll  127.15 (0.94x)  110.37 (0.87x)  107.97 (0.85x)   97.13 
(0.81x)   55.10 (0.86x)
GTree   Traverse  277.71  276.09  272.78  246.72
   98.47
QTree   Traverse  370.33 (1.33x)  411.97 (1.49x)  400.23 (1.47x)  262.82 
(1.07x)   78.52 (0.80x)


As a sanity check, the same benchmark when Glib's version
is >= $glib_dropped_gslice_version (i.e. QTree == GTree):

 Tree Op  3210244096  131072
 1048576

GTree Lookup   82.72   43.09   24.18   19.73
   16.09
QTree Lookup   81.82 (0.99x)   43.10 (1.00x)   24.20 (1.00x)   19.76 
(1.00x)   16.26 (1.01x)
GTree Insert   45.07   29.62   26.34   19.90
   17.18
QTree Insert   45.72 (1.01x)   29.60 (1.00x)   26.38 (1.00x)   19.71 
(0.99x)   17.20 (1.00x)
GTree Remove   54.48   35.36   31.77   24.97
   16.95
QTree Remove   54.46 (1.00x)   35.32 (1.00x)   31.77 (1.00x)   24.91 
(1.00x)   17.15 (1.01x)
GTree  RemoveAll  140.68  127.36  125.43  121.45
   68.20
QTree  RemoveAll  140.65 (1.00x)  127.64 (1.00x)  125.01 (1.00x)  121.73 
(1.00x)   67.06 (0.98x)
GTree   Traverse  278.68  276.05  266.75  251.65
  104.93
QTree   Traverse  278.31 (1.00x)  275.78 (1.00x)  266.42 (1.00x)  247.89 
(0.99x)  104.58 (1.00x)


Related: #285

Signed-off-by: Emilio Cota 
---
 configure |   15 +
 include/qemu/qtree.h  |  201 ++
 meson.build   |4 +
 tests/bench/meson.build   |4 +
 tests/bench/qtree-bench.c |  286 
 tests/unit/meson.build|1 +
 tests/unit/test-qtree.c   |  333 +
 util/meson.build  |1 +
 util/qtree.c  | 1390 +
 9 files changed, 2235 insertions(+)
 create mode 100644 include/qemu/qtree.h
 create mode 100644 tests/bench/qtree-bench.c
 create mode 100644 tests/unit/test-qtree.c
 create mode 100644 util/qtree.c

diff --git a/configure b/configure
index 64960c6000..8fe3d643c8 100755
--- a/configure
+++ b/configure
@@ -226,6 +226,7 @@ stack_protector=""
 safe_stack=""
 use_containers="yes"
 gdb_bin=$(command -v "gdb-multiarch" || command -v "gdb")
+glib_has_gslice="no"
 
 if test -e "$source_path/.git"
 then
@@ -1456,6 +1457,17 @@ for i in $glib_modules; do
 fi
 done
 
+# Check whether glib has gslice, which we have to avoid for correctness.
+# TODO: remove this check and the corresponding workaround (qtree) when
+# the minimum supported glib is >= $glib_dropped_gslice_version.
+glib_dropped_gslice_version=2.75.3
+for i in $glib_modules; do
+if ! $pkg_config --atleast-version=$glib_dropped_gslice_version $i; then
+glib_has_gslice="yes"
+   break
+fi
+done
+
 glib_bindir="$($pkg_config --v

Re: [PATCH 2/2] tcg: use QTree instead of GTree

2023-02-05 Thread Emilio Cota
On Mon, Jan 30, 2023 at 09:09:47 -1000, Richard Henderson wrote:
> On 1/29/23 23:27, Daniel P. Berrangé wrote:
> > On Sun, Jan 29, 2023 at 05:38:08PM -0500, Emilio Cota wrote:
> > > Since this is a correctness issue, I think we should ship with qtree
> > > and use it when configuring with glib <2.76.0. For later glib versions
> > > we would just use gtree, e.g. via typedef + inline functions.
> > > 
> > > Once the minimum glib required by the configure script is >= 2.76.0,
> > > then we'd remove qtree.
> > > 
> > > If that sounds like a good plan, I can send a v2.
> > 
> > I'm fine with it, but be good to have an opinion here from the TCG
> > subsystem maintainers, CC'ing them
> 
> I agree the correctness issue wants the fix now,
> and typedef + inlines sounds like a good way moving forward.

Thanks, just sent a v2 implementing the above.

Emilio



Re: [PULL 00/40] tcg patch queue

2023-02-05 Thread Peter Maydell
On Sat, 4 Feb 2023 at 16:33, Richard Henderson
 wrote:
>
> The following changes since commit 579510e196a544b42bd8bca9cc61688d4d1211ac:
>
>   Merge tag 'pull-monitor-2023-02-03-v2' of https://repo.or.cz/qemu/armbru 
> into staging (2023-02-04 10:19:55 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/rth7680/qemu.git tags/pull-tcg-20230204
>
> for you to fetch changes up to a2495ede07498ee36b18b03e7038ba30c9871bb2:
>
>   tcg/aarch64: Fix patching of LDR in tb_target_set_jmp_target (2023-02-04 
> 06:19:43 -1000)
>
> 
> tcg: Add support for TCGv_i128 in parameters and returns.
> tcg: Add support for TCGv_i128 in cmpxchg.
> tcg: Test CPUJumpCache in tb_jmp_cache_clear_page
> tcg: Split out tcg_gen_nonatomic_cmpxchg_i{32,64}
> tcg/aarch64: Fix patching of LDR in tb_target_set_jmp_target
> target/arm: Use tcg_gen_atomic_cmpxchg_i128
> target/i386: Use tcg_gen_atomic_cmpxchg_i128
> target/i386: Use tcg_gen_nonatomic_cmpxchg_i{32,64}
> target/s390x: Use tcg_gen_atomic_cmpxchg_i128
> target/s390x: Use TCGv_i128 in passing and returning float128
> target/s390x: Implement CC_OP_NZ in gen_op_calc_cc
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0
for any user-visible changes.

-- PMM



Re: pixman_blt on aarch64

2023-02-05 Thread Richard Henderson

On 2/4/23 06:57, BALATON Zoltan wrote:
This has just bounced, I hoped to still be able to post after moderation but now I'm 
resending it after subscribing to the pixman list. Meanwhile I've found this ticket as 
well: https://gitlab.freedesktop.org/pixman/pixman/-/merge_requests/71
See the rest of the message below. Looks like this is being worked on but I'm not sure how 
far is it from getting resolved. Any info on that?


Please try this:

https://gitlab.freedesktop.org/rth7680/pixman/-/tree/general

It provides a pure C version for ultimate fallback.
Unfortunately, there are no test cases for this, nor documentation.


r~



Re: pixman_blt on aarch64

2023-02-05 Thread BALATON Zoltan

On Sun, 5 Feb 2023, Richard Henderson wrote:

On 2/4/23 06:57, BALATON Zoltan wrote:
This has just bounced, I hoped to still be able to post after moderation 
but now I'm resending it after subscribing to the pixman list. Meanwhile 
I've found this ticket as well: 
https://gitlab.freedesktop.org/pixman/pixman/-/merge_requests/71
See the rest of the message below. Looks like this is being worked on but 
I'm not sure how far is it from getting resolved. Any info on that?


Please try this:

https://gitlab.freedesktop.org/rth7680/pixman/-/tree/general

It provides a pure C version for ultimate fallback.
Unfortunately, there are no test cases for this, nor documentation.


Thanks, I don't have hardware to test this but maybe Akihiko or somebody 
else here cam try. Do you think pixman_fill won't have the same problem? 
It seems to have at least a fast_path implementation but I'm not sure how 
pixman selects these.


Regards,
BALATON Zoltan



qemu-img hangs on s390x

2023-02-05 Thread Michael Tokarev

There's a bug filed against qemu on debian, about qemu-img hanging on s390x.
While digging in, I discovered that the thing is broken there indeed, and it
is broken for a very long time, and it is interesting.

The reproducer is rather simple:

 qemu-img create -f qcow2 -o preallocation=metadata blank-disk-1s.qcow2 512

this hangs until interrupted, after writing 327680 bytes of output.
I haven't tried old versions, - 5.2 hangs for sure, as is 7.2 and apparently
all in-between. In particular, current debian sid (whole thing) and 2-years
old debian bullseye hangs equally.

But the thing is that it does not hang when creating file on a tmpfs, -
when the filesystem is tmpfs, it always works.

Also, a few times I were able to run the above qemu-img create successfully, -
maybe 2 out of 100 runs or so.

It looks like the problem has been there for a very long time, and it is
timing-dependent.

Comparing strace of the two runs, I see differences in most futex operations.
Here's the parent process:

...
 read(7, "\0\0\0\0\0\0\0\1", 512)= 8
 ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = 1 ([{fd=7, 
revents=POLLIN}])
-futex(0x2aa29af8bb4, FUTEX_WAKE_PRIVATE, 1) = 1
+futex(0x2aa03600bb4, FUTEX_WAKE_PRIVATE, 1) = 0
 read(7, "\0\0\0\0\0\0\0\1", 512)= 8
+futex(0x2aa03600bb0, FUTEX_WAKE_PRIVATE, 1) = 1
 ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = 1 ([{fd=7, 
revents=POLLIN}])
-futex(0x2aa29af8bb0, FUTEX_WAKE_PRIVATE, 1) = 1
+futex(0x2aa03600bb4, FUTEX_WAKE_PRIVATE, 1) = 0
 read(7, "\0\0\0\0\0\0\0\1", 512)= 8
 ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = 1 ([{fd=7, 
revents=POLLIN}])
-futex(0x2aa29af8bb4, FUTEX_WAKE_PRIVATE, 1) = 1
+futex(0x2aa03600bb0, FUTEX_WAKE_PRIVATE, 1) = 0
 read(7, "\0\0\0\0\0\0\0\1", 512)= 8
 ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = 1 ([{fd=7, 
revents=POLLIN}])
-futex(0x2aa29af8bb0, FUTEX_WAKE_PRIVATE, 1) = 1
+futex(0x2aa03600bb4, FUTEX_WAKE_PRIVATE, 1) = 0
 read(7, "\0\0\0\0\0\0\0\1", 512)= 8
 ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = 1 ([{fd=7, 
revents=POLLIN}])
-futex(0x2aa29af8bb4, FUTEX_WAKE_PRIVATE, 1) = 0
+futex(0x2aa03600bb0, FUTEX_WAKE_PRIVATE, 1) = 1
 read(7, "\0\0\0\0\0\0\0\1", 512)= 8
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
-ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = ? 
ERESTARTNOHAND (To be restarted if no handler)
 ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = 1 ([{fd=7, 
revents=POLLIN}])
 SIGINT {si_signo=SIGINT, si_code=SI_KERNEL} ---
-+++ killed by SIGINT +++
+futex(0x2aa03600bb4, FUTEX_WAKE_PRIVATE, 1) = 0
+read(7, "\0\0\0\0\0\0\0\1", 512)= 8
+ppoll([{fd=7, events=POLLIN|POLLERR|POLLHUP}], 1, NULL, NULL, 8) = 1 ([{fd=7, 
revents=POLLIN}])
+futex(0x2aa03600bb0, FUTEX_WAKE_PRIVATE, 1) = 0
+read(7, "\0\0\0\0\0\0\0\1", 512)= 8
...

(I've hit Ctrl+C after quite some time).

I'll take another look at this tomorrow. But if someone knows
what's going on there, please tell me :)  The situation is quite
interesting, - is it possible we missed such a serious issue somehow?

Thanks,

/mjt



Re: pixman_blt on aarch64

2023-02-05 Thread Richard Henderson

On 2/5/23 08:44, BALATON Zoltan wrote:

On Sun, 5 Feb 2023, Richard Henderson wrote:

On 2/4/23 06:57, BALATON Zoltan wrote:
This has just bounced, I hoped to still be able to post after moderation but now I'm 
resending it after subscribing to the pixman list. Meanwhile I've found this ticket as 
well: https://gitlab.freedesktop.org/pixman/pixman/-/merge_requests/71
See the rest of the message below. Looks like this is being worked on but I'm not sure 
how far is it from getting resolved. Any info on that?


Please try this:

https://gitlab.freedesktop.org/rth7680/pixman/-/tree/general

It provides a pure C version for ultimate fallback.
Unfortunately, there are no test cases for this, nor documentation.


Thanks, I don't have hardware to test this but maybe Akihiko or somebody else here cam 
try. Do you think pixman_fill won't have the same problem? It seems to have at least a 
fast_path implementation but I'm not sure how pixman selects these.


For fill, I think the fast_path implementation should work, so long as it isn't disabled 
via environment variable.  I'm not sure why that is, and why _fast_path isn't part of 
_general.


Indeed, the fast_path implementation of fill should be easily vectorized by the compiler. 
I would expect it to be competitive with an assembly implementation.  I would expect the 
implementation chain design to only be useful when multiple vector implementations are 
supported and selected at runtime -- e.g. the x86 SSE2 vs SSSE3 stuff.



r~



Re: [PATCH v2 02/10] softmmu/ioport: Merge portio_list_add() into portio_list_init()

2023-02-05 Thread Mark Cave-Ayland

On 26/01/2023 21:17, Bernhard Beschow wrote:


Both functions are always used together and in the same order. Let's
reflect this in the API.

Inspired-by: <20210518215545.1793947-9-phi...@redhat.com>
   'hw/isa: Extract bus part from isa_register_portio_list()'
Signed-off-by: Bernhard Beschow 
---
  include/exec/ioport.h   |  6 ++
  hw/audio/adlib.c|  4 ++--
  hw/display/qxl.c|  5 ++---
  hw/display/vga.c|  8 
  hw/dma/i82374.c |  6 ++
  hw/isa/isa-bus.c|  6 ++
  hw/watchdog/wdt_ib700.c |  4 ++--
  softmmu/ioport.c| 19 +++
  8 files changed, 23 insertions(+), 35 deletions(-)

diff --git a/include/exec/ioport.h b/include/exec/ioport.h
index e34f668998..ec3e8e5942 100644
--- a/include/exec/ioport.h
+++ b/include/exec/ioport.h
@@ -64,12 +64,10 @@ typedef struct PortioList {
  
  void portio_list_init(PortioList *piolist, Object *owner,

const struct MemoryRegionPortio *callbacks,
-  void *opaque, const char *name);
+  void *opaque, const char *name,
+  MemoryRegion *address_space_io, uint16_t start);
  void portio_list_set_flush_coalesced(PortioList *piolist);
  void portio_list_destroy(PortioList *piolist);
-void portio_list_add(PortioList *piolist,
- struct MemoryRegion *address_space,
- uint32_t addr);
  void portio_list_del(PortioList *piolist);
  
  #endif /* IOPORT_H */

diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index 5f979b1487..957abe3da7 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -291,8 +291,8 @@ static void adlib_realizefn (DeviceState *dev, Error **errp)
  
  adlib_portio_list[0].offset = s->port;

  adlib_portio_list[1].offset = s->port + 8;
-portio_list_init (&s->port_list, OBJECT(s), adlib_portio_list, s, "adlib");
-portio_list_add (&s->port_list, isa_address_space_io(&s->parent_obj), 0);
+portio_list_init(&s->port_list, OBJECT(s), adlib_portio_list, s, "adlib",
+ isa_address_space_io(&s->parent_obj), 0);
  }
  
  static Property adlib_properties[] = {

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index ec712d3ca2..6d5a931425 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -2224,10 +2224,9 @@ static void qxl_realize_primary(PCIDevice *dev, Error 
**errp)
  }
  vga_init(vga, OBJECT(dev),
   pci_address_space(dev), pci_address_space_io(dev), false);
-portio_list_init(&qxl->vga_port_list, OBJECT(dev), qxl_vga_portio_list,
- vga, "vga");
+portio_list_init(&qxl->vga_port_list, OBJECT(dev), qxl_vga_portio_list, 
vga,
+ "vga", pci_address_space_io(dev), 0x3b0);
  portio_list_set_flush_coalesced(&qxl->vga_port_list);
-portio_list_add(&qxl->vga_port_list, pci_address_space_io(dev), 0x3b0);
  qxl->have_vga = true;
  
  vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl);

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 7a5fdff649..2b606a526c 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2309,12 +2309,12 @@ void vga_init(VGACommonState *s, Object *obj, 
MemoryRegion *address_space,
  1);
  memory_region_set_coalescing(vga_io_memory);
  if (init_vga_ports) {
-portio_list_init(&s->vga_port_list, obj, vga_ports, s, "vga");
+portio_list_init(&s->vga_port_list, obj, vga_ports, s, "vga",
+ address_space_io, 0x3b0);
  portio_list_set_flush_coalesced(&s->vga_port_list);
-portio_list_add(&s->vga_port_list, address_space_io, 0x3b0);
  }
  if (vbe_ports) {
-portio_list_init(&s->vbe_port_list, obj, vbe_ports, s, "vbe");
-portio_list_add(&s->vbe_port_list, address_space_io, 0x1ce);
+portio_list_init(&s->vbe_port_list, obj, vbe_ports, s, "vbe",
+ address_space_io, 0x1ce);
  }
  }
diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
index 34c3aaf7d3..5914b34079 100644
--- a/hw/dma/i82374.c
+++ b/hw/dma/i82374.c
@@ -131,10 +131,8 @@ static void i82374_realize(DeviceState *dev, Error **errp)
  }
  i8257_dma_init(isa_bus, true);
  
-portio_list_init(&s->port_list, OBJECT(s), i82374_portio_list, s,

- "i82374");
-portio_list_add(&s->port_list, isa_address_space_io(&s->parent_obj),
-s->iobase);
+portio_list_init(&s->port_list, OBJECT(s), i82374_portio_list, s, "i82374",
+ isa_address_space_io(&s->parent_obj), s->iobase);
  
  memset(s->commands, 0, sizeof(s->commands));

  }
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 1bee1a47f1..b3497dad61 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -124,8 +124,6 @@ int isa_register_portio_list(ISADevice *dev,
   const MemoryRegionPortio *pio_start,
   void *opaque, const char *name

Re: [PATCH v2 03/10] softmmu/ioport: Remove unused functions

2023-02-05 Thread Mark Cave-Ayland

On 26/01/2023 21:17, Bernhard Beschow wrote:


Signed-off-by: Bernhard Beschow 
---
  include/exec/ioport.h |  2 --
  softmmu/ioport.c  | 24 
  2 files changed, 26 deletions(-)

diff --git a/include/exec/ioport.h b/include/exec/ioport.h
index ec3e8e5942..1ef5aebba3 100644
--- a/include/exec/ioport.h
+++ b/include/exec/ioport.h
@@ -67,7 +67,5 @@ void portio_list_init(PortioList *piolist, Object *owner,
void *opaque, const char *name,
MemoryRegion *address_space_io, uint16_t start);
  void portio_list_set_flush_coalesced(PortioList *piolist);
-void portio_list_destroy(PortioList *piolist);
-void portio_list_del(PortioList *piolist);
  
  #endif /* IOPORT_H */

diff --git a/softmmu/ioport.c b/softmmu/ioport.c
index c92e3cb27d..0a55d39196 100644
--- a/softmmu/ioport.c
+++ b/softmmu/ioport.c
@@ -118,19 +118,6 @@ void portio_list_set_flush_coalesced(PortioList *piolist)
  piolist->flush_coalesced_mmio = true;
  }
  
-void portio_list_destroy(PortioList *piolist)

-{
-MemoryRegionPortioList *mrpio;
-unsigned i;
-
-for (i = 0; i < piolist->nr; ++i) {
-mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, mr);
-object_unparent(OBJECT(&mrpio->mr));
-g_free(mrpio);
-}
-g_free(piolist->regions);
-}
-
  static const MemoryRegionPortio *find_portio(MemoryRegionPortioList *mrpio,
   uint64_t offset, unsigned size,
   bool write)
@@ -280,14 +267,3 @@ void portio_list_init(PortioList *piolist, Object *owner,
  /* There will always be an open sub-list.  */
  portio_list_add_1(piolist, pio_start, count, start, off_low, off_high);
  }
-
-void portio_list_del(PortioList *piolist)
-{
-MemoryRegionPortioList *mrpio;
-unsigned i;
-
-for (i = 0; i < piolist->nr; ++i) {
-mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, mr);
-memory_region_del_subregion(piolist->address_space, &mrpio->mr);
-}
-}


I think it may be worth leaving these functions. There were previous discussions 
around the cmd646 and via PCI-IDE interfaces which have a bit in PCI configuration 
space that switches the chip between compatibility (ISA) mode and PCI mode. I could 
see that switching the device to PCI mode would require removal of the old ISA ports, 
for example, as in PCI mode the registers would be accessed exclusively via the PCI BAR.



ATB,

Mark.



Re: [PATCH v2 04/10] hw/ide/piix: Disuse isa_get_irq()

2023-02-05 Thread Mark Cave-Ayland

On 26/01/2023 21:17, Bernhard Beschow wrote:


isa_get_irq() asks for an ISADevice which piix-ide doesn't provide.
Passing a NULL pointer works but causes the isabus global to be used
then. By fishing out TYPE_ISA_BUS from the QOM tree it is possible to
achieve the same as isa_get_irq().

This is an alternative solution to commit 9405d87be25d 'hw/ide: Fix
crash when plugging a piix3-ide device into the x-remote machine' which
allows for cleaning up the ISA API while keeping PIIX IDE functions
user-createable.

Signed-off-by: Bernhard Beschow 
---
  hw/ide/piix.c | 21 ++---
  1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 267dbf37db..a6646d9657 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -126,7 +126,7 @@ static void piix_ide_reset(DeviceState *dev)
  pci_set_byte(pci_conf + 0x20, 0x01);  /* BMIBA: 20-23h */
  }
  
-static int pci_piix_init_ports(PCIIDEState *d)

+static int pci_piix_init_ports(PCIIDEState *d, ISABus *isa_bus)
  {
  static const struct {
  int iobase;
@@ -145,7 +145,7 @@ static int pci_piix_init_ports(PCIIDEState *d)
  if (ret) {
  return ret;
  }
-ide_init2(&d->bus[i], isa_get_irq(NULL, port_info[i].isairq));
+ide_init2(&d->bus[i], isa_bus->irqs[port_info[i].isairq]);
  
  bmdma_init(&d->bus[i], &d->bmdma[i], d);

  d->bmdma[i].bus = &d->bus[i];
@@ -159,6 +159,8 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error 
**errp)
  {
  PCIIDEState *d = PCI_IDE(dev);
  uint8_t *pci_conf = dev->config;
+ISABus *isa_bus;
+bool ambiguous;
  int rc;
  
  pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode

@@ -168,7 +170,20 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error 
**errp)
  
  vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
  
-rc = pci_piix_init_ports(d);

+isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous));
+if (ambiguous) {
+error_setg(errp,
+   "More than one ISA bus found while %s supports only one",
+   object_get_typename(OBJECT(dev)));
+return;
+}
+if (!isa_bus) {
+error_setg(errp, "No ISA bus found while %s requires one",
+   object_get_typename(OBJECT(dev)));
+return;
+}
+
+rc = pci_piix_init_ports(d, isa_bus);
  if (rc) {
  error_setg_errno(errp, -rc, "Failed to realize %s",
   object_get_typename(OBJECT(dev)));


I think the approach here to allow the PCI-ISA bridge to locate the ISABus is a good 
one, but I think it's worth keeping isa_get_irq() to avoid exposing the internals to 
devices.


For me the problem here is that isa_get_irq() accepts a NULL argument for ISADevice. 
I'd expect the function to look something like isa_bus_get_irq(ISABus *isa_bus, 
unsigned isairq) and then it is the responsibility of the caller to locate and 
specify the correct ISABus to avoid falling back to the isabus global.


In particular I can see a PCIDevice should be able to attempt a pci_get_isa_bus() or 
similar which would locate the PCI-ISA bridge and return the child ISA bus, which 
again is related to the cmd646/via compatibility mode feature.


This is along the lines of the similar approach I discussed in 
https://lists.gnu.org/archive/html/qemu-devel/2022-12/msg01443.html which is an 
evolution of the ideas discussed in the original thread a couple of years earlier at 
https://lists.nongnu.org/archive/html/qemu-devel/2020-03/msg01707.html.



ATB,

Mark.



Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances

2023-02-05 Thread Mark Cave-Ayland

On 26/01/2023 21:17, Bernhard Beschow wrote:


Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.

Signed-off-by: Bernhard Beschow 
---
  include/hw/ide/pci.h |  1 +
  hw/ide/piix.c| 64 ++--
  hw/isa/piix.c|  5 
  3 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
  MemoryRegion bmdma_bar;
  MemoryRegion cmd_bar[2];
  MemoryRegion data_bar[2];
+bool user_created;
  };
  
  static inline IDEState *bmdma_active_if(BMDMAState *bmdma)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
  }
  }
  
+static void piix_ide_set_irq(void *opaque, int n, int level)

+{
+PCIIDEState *d = opaque;
+
+qemu_set_irq(d->isa_irqs[n], level);
+}
+
  static void piix_ide_reset(DeviceState *dev)
  {
  PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d, ISABus 
*isa_bus)
  };
  int i;
  
+if (isa_bus) {

+d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+} else {
+qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+}
+
  for (i = 0; i < 2; i++) {
  ide_bus_init(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
  ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
  port_info[i].iobase2);
-ide_init2(&d->bus[i], isa_bus->irqs[port_info[i].isairq]);
+ide_init2(&d->bus[i], qdev_get_gpio_in(DEVICE(d), i));
  
  bmdma_init(&d->bus[i], &d->bmdma[i], d);

  d->bmdma[i].bus = &d->bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error 
**errp)
  {
  PCIIDEState *d = PCI_IDE(dev);
  uint8_t *pci_conf = dev->config;
-ISABus *isa_bus;
-bool ambiguous;
+ISABus *isa_bus = NULL;
  
  pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
  
@@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error **errp)
  
  vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
  
-isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous));

-if (ambiguous) {
-error_setg(errp,
-   "More than one ISA bus found while %s supports only one",
-   object_get_typename(OBJECT(dev)));
-return;
-}
-if (!isa_bus) {
-error_setg(errp, "No ISA bus found while %s requires one",
-   object_get_typename(OBJECT(dev)));
-return;
+if (d->user_created) {
+bool ambiguous;
+
+isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+   &ambiguous));
+
+if (ambiguous) {
+error_setg(errp,
+   "More than one ISA bus found while %s supports only 
one",
+   object_get_typename(OBJECT(dev)));
+return;
+}
+
+if (!isa_bus) {
+error_setg(errp, "No ISA bus found while %s requires one",
+   object_get_typename(OBJECT(dev)));
+return;
+}
  }
  
  pci_piix_init_ports(d, isa_bus);

  }
  
+static void pci_piix_ide_init(Object *obj)

+{
+DeviceState *dev = DEVICE(obj);
+
+qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
+}
+
  static void pci_piix_ide_exitfn(PCIDevice *dev)
  {
  PCIIDEState *d = PCI_IDE(dev);
@@ -191,6 +218,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
  }
  }
  
+static Property piix_ide_properties[] = {

+DEFINE_PROP_BOOL("user-created", PCIIDEState, user_created, true),
+DEFINE_PROP_END_OF_LIST(),
+};
+
  /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
  static void piix3_ide_class_init(ObjectClass *klass, void *data)
  {
@@ -205,11 +237,13 @@ static void piix3_ide_class_init(ObjectClass *klass, void 
*data)
  k->class_id = PCI_CLASS_STORAGE_IDE;
  set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
  dc->hotpluggable = false;
+device_class_set_props(dc, piix_ide_properties);
  }
  
  static const TypeInfo piix3_ide_info = {

  .name  = TYPE_PIIX3_IDE,
  .parent= TYPE_PCI_IDE,
+.instance_init = pci_piix_ide_init,
  .class_init= piix3_ide_class_init,
  };
  
@@ -227,11 +261,13 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data)

  k->class_id = P

Re: [PATCH v2 08/10] hw/ide: Let ide_init_ioport() take a MemoryRegion argument instead of ISADevice

2023-02-05 Thread Mark Cave-Ayland

On 26/01/2023 21:17, Bernhard Beschow wrote:


Both callers to ide_init_ioport() have access to the I/O memory region
of the ISA bus, so can pass it directly. This allows ide_init_ioport()
to directly call portio_list_init().

Note, now the callers become the owner of the PortioList.

Inspired-by: <20210518215545.1793947-10-phi...@redhat.com>
   'hw/ide: Let ide_init_ioport() take an ISA bus argument instead of device'
Signed-off-by: Bernhard Beschow 
---
  include/hw/ide/internal.h |  3 ++-
  hw/ide/ioport.c   | 15 ---
  hw/ide/isa.c  |  4 +++-
  hw/ide/piix.c |  8 ++--
  4 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 42c49414f4..c3e4d192fa 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -628,7 +628,8 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, 
IDEDriveKind kind,
 int chs_trans, Error **errp);
  void ide_init2(IDEBus *bus, qemu_irq irq);
  void ide_exit(IDEState *s);
-void ide_init_ioport(IDEBus *bus, ISADevice *isa, int iobase, int iobase2);
+void ide_init_ioport(IDEBus *bus, MemoryRegion *address_space_io, Object 
*owner,
+ int iobase, int iobase2);
  void ide_register_restart_cb(IDEBus *bus);
  
  void ide_exec_cmd(IDEBus *bus, uint32_t val);

diff --git a/hw/ide/ioport.c b/hw/ide/ioport.c
index b613ff3bba..00e9baf0d1 100644
--- a/hw/ide/ioport.c
+++ b/hw/ide/ioport.c
@@ -50,15 +50,16 @@ static const MemoryRegionPortio ide_portio2_list[] = {
  PORTIO_END_OF_LIST(),
  };
  
-void ide_init_ioport(IDEBus *bus, ISADevice *dev, int iobase, int iobase2)

+void ide_init_ioport(IDEBus *bus, MemoryRegion *address_space_io, Object 
*owner,
+ int iobase, int iobase2)
  {
-/* ??? Assume only ISA and PCI configurations, and that the PCI-ISA
-   bridge has been setup properly to always register with ISA.  */
-isa_register_portio_list(dev, &bus->portio_list,
- iobase, ide_portio_list, bus, "ide");
+assert(address_space_io);
+
+portio_list_init(&bus->portio_list, owner, ide_portio_list, bus, "ide",
+ address_space_io, iobase);
  
  if (iobase2) {

-isa_register_portio_list(dev, &bus->portio2_list,
- iobase2, ide_portio2_list, bus, "ide");
+portio_list_init(&bus->portio2_list, owner, ide_portio2_list, bus,
+ "ide", address_space_io, iobase2);
  }
  }
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 8bedbd13f1..cab5d0a07a 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -72,9 +72,11 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp)
  {
  ISADevice *isadev = ISA_DEVICE(dev);
  ISAIDEState *s = ISA_IDE(dev);
+ISABus *isabus = isa_bus_from_device(isadev);
  
  ide_bus_init(&s->bus, sizeof(s->bus), dev, 0, 2);

-ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2);
+ide_init_ioport(&s->bus, isabus->address_space_io, OBJECT(dev),
+s->iobase, s->iobase2);
  s->irq = isa_get_irq(isadev, s->isairq);
  ide_init2(&s->bus, s->irq);
  vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index f0d95761ac..236b5b7416 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -29,6 +29,7 @@
  
  #include "qemu/osdep.h"

  #include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
  #include "migration/vmstate.h"
  #include "qapi/error.h"
  #include "qemu/module.h"
@@ -143,8 +144,11 @@ static void pci_piix_init_ports(PCIIDEState *d, ISABus 
*isa_bus)
  {0x1f0, 0x3f6, 14},
  {0x170, 0x376, 15},
  };
+PCIBus *pci_bus = pci_get_bus(&d->parent_obj);
  int i;
  
+assert(pci_bus);

+
  if (isa_bus) {
  d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
  d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
@@ -154,8 +158,8 @@ static void pci_piix_init_ports(PCIIDEState *d, ISABus 
*isa_bus)
  
  for (i = 0; i < 2; i++) {

  ide_bus_init(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
-ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
-port_info[i].iobase2);
+ide_init_ioport(&d->bus[i], pci_bus->address_space_io, OBJECT(d),
+port_info[i].iobase, port_info[i].iobase2);
  ide_init2(&d->bus[i], qdev_get_gpio_in(DEVICE(d), i));
  
  bmdma_init(&d->bus[i], &d->bmdma[i], d);


Again, given that I suspect ioports are specific to x86 I'd be inclined to leave this 
as a reference to ISA. I could see there being a function that exists such as 
isa_get_address_space_io(ISADevice *isa) in the same way as pci_address_space_io(), 
for example.



ATB,

Mark.



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-05 Thread Mark Cave-Ayland

On 30/01/2023 20:45, Alex Bennée wrote:


Daniel P. Berrangé  writes:


On Mon, Jan 30, 2023 at 11:47:02AM +, Peter Maydell wrote:

On Mon, 30 Jan 2023 at 11:44, Thomas Huth  wrote:


Testing 32-bit host OS support takes a lot of precious time during the QEMU
contiguous integration tests, and considering that many OS vendors stopped
shipping 32-bit variants of their OS distributions and most hardware from
the past >10 years is capable of 64-bit


True for x86, not necessarily true for other architectures.
Are you proposing to deprecate x86 32-bit, or all 32-bit?
I'm not entirely sure about whether we're yet at a point where
I'd want to deprecate-and-drop 32-bit arm host support.


Do we have a feeling on which aspects of 32-bit cause us the support
burden ? The boring stuff like compiler errors from mismatched integer
sizes is mostly quick & easy to detect simply through a cross compile.

I vaguely recall someone mentioned problems with atomic ops in the past,
or was it 128-bit ints, caused implications for the codebase ?


Atomic operations on > TARGET_BIT_SIZE and cputlb when
TCG_OVERSIZED_GUEST is set. Also the core TCG code and a bunch of the
backends have TARGET_LONG_BITS > TCG_TARGET_REG_BITS ifdefs peppered
throughout.


I am one of an admittedly small group of people still interested in using KVM-PR on 
ppc32 to boot MacOS, although there is some interest on using 64-bit KVM-PR to run 
super-fast MacOS on modern Talos hardware.


From my perspective losing the ability to run 64-bit guests on 32-bit hardware with 
TCG wouldn't be an issue, as long as it were still possible to use qemu-system-ppc on 
32-bit hardware using both TCG and KVM to help debug the remaining issues.



ATB,

Mark.



Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances

2023-02-05 Thread BALATON Zoltan

On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:

On 26/01/2023 21:17, Bernhard Beschow wrote:

Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.

Signed-off-by: Bernhard Beschow 
---
  include/hw/ide/pci.h |  1 +
  hw/ide/piix.c| 64 ++--
  hw/isa/piix.c|  5 
  3 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
  MemoryRegion bmdma_bar;
  MemoryRegion cmd_bar[2];
  MemoryRegion data_bar[2];
+bool user_created;
  };
static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
  }
  }
  +static void piix_ide_set_irq(void *opaque, int n, int level)
+{
+PCIIDEState *d = opaque;
+
+qemu_set_irq(d->isa_irqs[n], level);
+}
+
  static void piix_ide_reset(DeviceState *dev)
  {
  PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d, 
ISABus *isa_bus)

  };
  int i;
  +if (isa_bus) {
+d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+} else {
+qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+}
+
  for (i = 0; i < 2; i++) {
  ide_bus_init(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
  ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
  port_info[i].iobase2);
-ide_init2(&d->bus[i], isa_bus->irqs[port_info[i].isairq]);
+ide_init2(&d->bus[i], qdev_get_gpio_in(DEVICE(d), i));
bmdma_init(&d->bus[i], &d->bmdma[i], d);
  d->bmdma[i].bus = &d->bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error 
**errp)

  {
  PCIIDEState *d = PCI_IDE(dev);
  uint8_t *pci_conf = dev->config;
-ISABus *isa_bus;
-bool ambiguous;
+ISABus *isa_bus = NULL;
pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
  @@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice *dev, 
Error **errp)

vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
  -isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS, 
&ambiguous));

-if (ambiguous) {
-error_setg(errp,
-   "More than one ISA bus found while %s supports only 
one",

-   object_get_typename(OBJECT(dev)));
-return;
-}
-if (!isa_bus) {
-error_setg(errp, "No ISA bus found while %s requires one",
-   object_get_typename(OBJECT(dev)));
-return;
+if (d->user_created) {
+bool ambiguous;
+
+isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+   &ambiguous));
+
+if (ambiguous) {
+error_setg(errp,
+   "More than one ISA bus found while %s supports only 
one",

+   object_get_typename(OBJECT(dev)));
+return;
+}
+
+if (!isa_bus) {
+error_setg(errp, "No ISA bus found while %s requires one",
+   object_get_typename(OBJECT(dev)));
+return;
+}
  }
pci_piix_init_ports(d, isa_bus);
  }
  +static void pci_piix_ide_init(Object *obj)
+{
+DeviceState *dev = DEVICE(obj);
+
+qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
+}
+
  static void pci_piix_ide_exitfn(PCIDevice *dev)
  {
  PCIIDEState *d = PCI_IDE(dev);
@@ -191,6 +218,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
  }
  }
  +static Property piix_ide_properties[] = {
+DEFINE_PROP_BOOL("user-created", PCIIDEState, user_created, true),
+DEFINE_PROP_END_OF_LIST(),
+};
+
  /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
  static void piix3_ide_class_init(ObjectClass *klass, void *data)
  {
@@ -205,11 +237,13 @@ static void piix3_ide_class_init(ObjectClass *klass, 
void *data)

  k->class_id = PCI_CLASS_STORAGE_IDE;
  set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
  dc->hotpluggable = false;
+device_class_set_props(dc, piix_ide_properties);
  }
static const TypeInfo piix3_ide_info = {
  .name  = TYPE_PIIX3_IDE,
  .parent= TYPE_PCI_IDE,
+.instance_init = pci_piix_ide_init,
  .class_init= piix3_ide_class_init,
  };
  @@ -227,11 +261,13 @@ static void piix4_ide_class_init(ObjectClass 
*klass, v

Re: [PATCH v2 07/10] hw/ide/piix: Require an ISABus only for user-created instances

2023-02-05 Thread Mark Cave-Ayland

On 05/02/2023 22:21, BALATON Zoltan wrote:


On Sun, 5 Feb 2023, Mark Cave-Ayland wrote:

On 26/01/2023 21:17, Bernhard Beschow wrote:

Internal instances now defer interrupt wiring to the caller which
decouples them from the ISABus. User-created devices still fish out the
ISABus from the QOM tree and the interrupt wiring remains in PIIX IDE.
The latter mechanism is considered a workaround and intended to be
removed once a deprecation period for user-created PIIX IDE devices is
over.

Signed-off-by: Bernhard Beschow 
---
  include/hw/ide/pci.h |  1 +
  hw/ide/piix.c    | 64 ++--
  hw/isa/piix.c    |  5 
  3 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 24c0b7a2dd..ee2c8781b7 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -54,6 +54,7 @@ struct PCIIDEState {
  MemoryRegion bmdma_bar;
  MemoryRegion cmd_bar[2];
  MemoryRegion data_bar[2];
+    bool user_created;
  };
    static inline IDEState *bmdma_active_if(BMDMAState *bmdma)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 5980045db0..f0d95761ac 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -108,6 +108,13 @@ static void bmdma_setup_bar(PCIIDEState *d)
  }
  }
  +static void piix_ide_set_irq(void *opaque, int n, int level)
+{
+    PCIIDEState *d = opaque;
+
+    qemu_set_irq(d->isa_irqs[n], level);
+}
+
  static void piix_ide_reset(DeviceState *dev)
  {
  PCIIDEState *d = PCI_IDE(dev);
@@ -138,11 +145,18 @@ static void pci_piix_init_ports(PCIIDEState *d, ISABus 
*isa_bus)

  };
  int i;
  +    if (isa_bus) {
+    d->isa_irqs[0] = isa_bus->irqs[port_info[0].isairq];
+    d->isa_irqs[1] = isa_bus->irqs[port_info[1].isairq];
+    } else {
+    qdev_init_gpio_out(DEVICE(d), d->isa_irqs, 2);
+    }
+
  for (i = 0; i < 2; i++) {
  ide_bus_init(&d->bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
  ide_init_ioport(&d->bus[i], NULL, port_info[i].iobase,
  port_info[i].iobase2);
-    ide_init2(&d->bus[i], isa_bus->irqs[port_info[i].isairq]);
+    ide_init2(&d->bus[i], qdev_get_gpio_in(DEVICE(d), i));
    bmdma_init(&d->bus[i], &d->bmdma[i], d);
  d->bmdma[i].bus = &d->bus[i];
@@ -154,8 +168,7 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error 
**errp)
  {
  PCIIDEState *d = PCI_IDE(dev);
  uint8_t *pci_conf = dev->config;
-    ISABus *isa_bus;
-    bool ambiguous;
+    ISABus *isa_bus = NULL;
    pci_conf[PCI_CLASS_PROG] = 0x80; // legacy ATA mode
  @@ -164,22 +177,36 @@ static void pci_piix_ide_realize(PCIDevice *dev, Error 
**errp)

    vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_pci, d);
  -    isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS, 
&ambiguous));
-    if (ambiguous) {
-    error_setg(errp,
-   "More than one ISA bus found while %s supports only one",
-   object_get_typename(OBJECT(dev)));
-    return;
-    }
-    if (!isa_bus) {
-    error_setg(errp, "No ISA bus found while %s requires one",
-   object_get_typename(OBJECT(dev)));
-    return;
+    if (d->user_created) {
+    bool ambiguous;
+
+    isa_bus = ISA_BUS(object_resolve_path_type("", TYPE_ISA_BUS,
+   &ambiguous));
+
+    if (ambiguous) {
+    error_setg(errp,
+   "More than one ISA bus found while %s supports only 
one",
+   object_get_typename(OBJECT(dev)));
+    return;
+    }
+
+    if (!isa_bus) {
+    error_setg(errp, "No ISA bus found while %s requires one",
+   object_get_typename(OBJECT(dev)));
+    return;
+    }
  }
    pci_piix_init_ports(d, isa_bus);
  }
  +static void pci_piix_ide_init(Object *obj)
+{
+    DeviceState *dev = DEVICE(obj);
+
+    qdev_init_gpio_in(dev, piix_ide_set_irq, 2);
+}
+
  static void pci_piix_ide_exitfn(PCIDevice *dev)
  {
  PCIIDEState *d = PCI_IDE(dev);
@@ -191,6 +218,11 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
  }
  }
  +static Property piix_ide_properties[] = {
+    DEFINE_PROP_BOOL("user-created", PCIIDEState, user_created, true),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
  /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
  static void piix3_ide_class_init(ObjectClass *klass, void *data)
  {
@@ -205,11 +237,13 @@ static void piix3_ide_class_init(ObjectClass *klass, void 
*data)

  k->class_id = PCI_CLASS_STORAGE_IDE;
  set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
  dc->hotpluggable = false;
+    device_class_set_props(dc, piix_ide_properties);
  }
    static const TypeInfo piix3_ide_info = {
  .name  = TYPE_PIIX3_IDE,
  .parent    = TYPE_PCI_IDE,
+    .instance_init = pci_piix_ide_init,
  .class_init    = piix3_ide_class_init,
  };
  @@ -227,11 +261,13 @@ static void pii

Re: [PATCH 02/19] hw/qdev: Introduce qdev_prop_set_link()

2023-02-05 Thread Mark Cave-Ayland

On 03/02/2023 18:08, Philippe Mathieu-Daudé wrote:


Introduce qdev_prop_set_link(), equivalent of
object_property_set_link() for QDev objects.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/core/qdev-properties.c| 5 +
  include/hw/qdev-properties.h | 1 +
  2 files changed, 6 insertions(+)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 9789a2f5de..46236b1542 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -694,6 +694,11 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, 
Object *obj,
  }
  }
  
+void qdev_prop_set_link(DeviceState *dev, const char *name, Object *value)

+{
+object_property_set_link(OBJECT(dev), name, value, &error_abort);
+}
+
  void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
  {
  object_property_set_bool(OBJECT(dev), name, value, &error_abort);
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 21f399e9a3..c16dbefb2f 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -191,6 +191,7 @@ bool qdev_prop_set_drive_err(DeviceState *dev, const char 
*name,
   * Set properties between creation and realization.
   * @value must be valid.  Each property may be set at most once.
   */
+void qdev_prop_set_link(DeviceState *dev, const char *name, Object *value);
  void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
  void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
  void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);


A general comment from me on this one: my feeling is that the main difference between 
QOM properties and qdev properties is that qdev properties are exposed to the user 
(for example they appear in the output of "-device foo,help") compared to QOM 
properties which tend to be used internally.


Following this thinking I'd always envisaged that an implementation of 
qdev_prop_set_link() would also be exposed to command line users so that you could 
set link properties from the command line similar to this:


  -device lance,id=lance0 -device ledma,dma=lance0

Of course this won't work in its current form (we don't have implicit ids for 
in-built devices as a starting point), but it does fit in with the recent discussions 
re: building machines completely from scratch. Certainly it feels to me as if this 
should be clarified before going ahead with a full-scale conversion for link 
properties as per this and your other related series.



ATB,

Mark.



Re: [PATCH v2] KVM: dirty ring: check if vcpu is created before dirty_ring_reap_one

2023-02-05 Thread Peter Xu
Hi, Weinan,

On Sun, Feb 05, 2023 at 06:45:45AM +, Weinan Liu wrote:
> Failed to assert '(dirty_gfns && ring_size)' in kvm_dirty_ring_reap_one if
> the vcpu has not been finished to create yet. This bug occasionally occurs
> when I open 200+ qemu instances on my 16G 6-cores x86 machine. And it must
> be triggered if inserting a 'sleep(10)' into kvm_vcpu_thread_fn as below--
> 
>  static void *kvm_vcpu_thread_fn(void *arg)
>  {
>  CPUState *cpu = arg;
>  int r;
> 
>  rcu_register_thread();
> 
> +sleep(10);
>  qemu_mutex_lock_iothread();
>  qemu_thread_get_self(cpu->thread);
>  cpu->thread_id = qemu_get_thread_id();
>  cpu->can_do_io = 1;
> 
> where dirty ring reaper will wakeup but then a vcpu has not been finished
> to create.
> 
> Signed-off-by: Weinan Liu 
> ---
>  accel/kvm/kvm-all.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 7e6a6076b1..0070ad72b8 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -1416,6 +1416,11 @@ static void *kvm_dirty_ring_reaper_thread(void *data)
>   */
>  sleep(1);
>  
> +/* ensure kvm_init_vcpu is finished, so cpu->kvm_dirty_gfns is ok */
> +if (!phase_check(PHASE_MACHINE_READY)) {
> +continue;
> +}
> +

Here's an old patch for this:

https://lore.kernel.org/all/20220927154653.77296-1-pet...@redhat.com/

IMHO that one will be more straightforward and self contained than this
one.  What do you think?

When posting new patches, please also remember to copy maintainers.  For
this one, it's:

$ ./scripts/get_maintainer.pl -f accel/kvm/kvm-all.c 
Paolo Bonzini  (supporter:Overall KVM CPUs)

Thanks,

-- 
Peter Xu




Re: [PATCH] kvm: dirty-ring: Fix race with vcpu creation

2023-02-05 Thread Peter Xu
Ping

On Tue, Sep 27, 2022 at 11:46:53AM -0400, Peter Xu wrote:
> It's possible that we want to reap a dirty ring on a vcpu that is during
> creation, because the vcpu is put onto list (CPU_FOREACH visible) before
> initialization of the structures.  In this case:
> 
> qemu_init_vcpu
> x86_cpu_realizefn
> cpu_exec_realizefn
> cpu_list_add  < can be probed by CPU_FOREACH
> qemu_init_vcpu
> cpus_accel->create_vcpu_thread(cpu);
> kvm_init_vcpu
> map kvm_dirty_gfns  <--- kvm_dirty_gfns valid
> 
> Don't try to reap dirty ring on vcpus during creation or it'll crash.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2124756
> Reported-by: Xiaohui Li 
> Signed-off-by: Peter Xu 
> ---
>  accel/kvm/kvm-all.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 5acab1767f..df5fabd3a8 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -757,6 +757,15 @@ static uint32_t kvm_dirty_ring_reap_one(KVMState *s, 
> CPUState *cpu)
>  uint32_t ring_size = s->kvm_dirty_ring_size;
>  uint32_t count = 0, fetch = cpu->kvm_fetch_index;
>  
> +/*
> + * It's possible that we race with vcpu creation code where the vcpu is
> + * put onto the vcpus list but not yet initialized the dirty ring
> + * structures.  If so, skip it.
> + */
> +if (!cpu->created) {
> +return 0;
> +}
> +
>  assert(dirty_gfns && ring_size);
>  trace_kvm_dirty_ring_reap_vcpu(cpu->cpu_index);
>  
> -- 
> 2.37.3
> 

-- 
Peter Xu




Re: [PATCH v10 4/5] riscv: Introduce satp mode hw capabilities

2023-02-05 Thread Alistair Francis
On Fri, Feb 3, 2023 at 4:04 PM Alexandre Ghiti  wrote:
>
> Currently, the max satp mode is set with the only constraint that it must be
> implemented in QEMU, i.e. set in valid_vm_1_10_[32|64].
>
> But we actually need to add another level of constraint: what the hw is
> actually capable of, because currently, a linux booting on a sifive-u54
> boots in sv57 mode which is incompatible with the cpu's sv39 max
> capability.
>
> So add a new bitmap to RISCVSATPMap which contains this capability and
> initialize it in every XXX_cpu_init.
>
> Finally:
> - valid_vm_1_10_[32|64] constrains which satp mode the CPU can use
> - the CPU hw capabilities constrains what the user may select
> - the user's selection then constrains what's available to the guest
>   OS.
>
> Signed-off-by: Alexandre Ghiti 
> Reviewed-by: Andrew Jones 
> Reviewed-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c | 91 +-
>  target/riscv/cpu.h |  8 +++-
>  2 files changed, 72 insertions(+), 27 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 56057cf87c..7e9924ede9 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -293,18 +293,24 @@ const char *satp_mode_str(uint8_t satp_mode, bool 
> is_32_bit)
>  g_assert_not_reached();
>  }
>
> -/* Sets the satp mode to the max supported */
> -static void set_satp_mode_default_map(RISCVCPU *cpu)
> +static void set_satp_mode_max_supported(RISCVCPU *cpu,
> +uint8_t satp_mode)
>  {
>  bool rv32 = riscv_cpu_mxl(&cpu->env) == MXL_RV32;
> +const bool *valid_vm = rv32 ? valid_vm_1_10_32 : valid_vm_1_10_64;
>
> -if (riscv_feature(&cpu->env, RISCV_FEATURE_MMU)) {
> -cpu->cfg.satp_mode.map |=
> -(1 << satp_mode_from_str(rv32 ? "sv32" : "sv57"));
> -} else {
> -cpu->cfg.satp_mode.map |= (1 << satp_mode_from_str("mbare"));
> +for (int i = 0; i <= satp_mode; ++i) {
> +if (valid_vm[i]) {
> +cpu->cfg.satp_mode.supported |= (1 << i);
> +}
>  }
>  }
> +
> +/* Set the satp mode to the max supported */
> +static void set_satp_mode_default_map(RISCVCPU *cpu)
> +{
> +cpu->cfg.satp_mode.map = cpu->cfg.satp_mode.supported;
> +}
>  #endif
>
>  static void riscv_any_cpu_init(Object *obj)
> @@ -315,6 +321,13 @@ static void riscv_any_cpu_init(Object *obj)
>  #elif defined(TARGET_RISCV64)
>  set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
>  #endif
> +
> +#ifndef CONFIG_USER_ONLY
> +set_satp_mode_max_supported(RISCV_CPU(obj),
> +riscv_cpu_mxl(&RISCV_CPU(obj)->env) == MXL_RV32 ?
> +VM_1_10_SV32 : VM_1_10_SV57);
> +#endif
> +
>  set_priv_version(env, PRIV_VERSION_1_12_0);
>  register_cpu_props(obj);
>  }
> @@ -328,6 +341,9 @@ static void rv64_base_cpu_init(Object *obj)
>  register_cpu_props(obj);
>  /* Set latest version of privileged specification */
>  set_priv_version(env, PRIV_VERSION_1_12_0);
> +#ifndef CONFIG_USER_ONLY
> +set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
> +#endif
>  }
>
>  static void rv64_sifive_u_cpu_init(Object *obj)
> @@ -335,6 +351,9 @@ static void rv64_sifive_u_cpu_init(Object *obj)
>  CPURISCVState *env = &RISCV_CPU(obj)->env;
>  set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
>  set_priv_version(env, PRIV_VERSION_1_10_0);
> +#ifndef CONFIG_USER_ONLY
> +set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV39);
> +#endif
>  }
>
>  static void rv64_sifive_e_cpu_init(Object *obj)
> @@ -345,6 +364,9 @@ static void rv64_sifive_e_cpu_init(Object *obj)
>  set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
>  set_priv_version(env, PRIV_VERSION_1_10_0);
>  cpu->cfg.mmu = false;
> +#ifndef CONFIG_USER_ONLY
> +set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> +#endif
>  }
>
>  static void rv128_base_cpu_init(Object *obj)
> @@ -361,6 +383,9 @@ static void rv128_base_cpu_init(Object *obj)
>  register_cpu_props(obj);
>  /* Set latest version of privileged specification */
>  set_priv_version(env, PRIV_VERSION_1_12_0);
> +#ifndef CONFIG_USER_ONLY
> +set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
> +#endif
>  }
>  #else
>  static void rv32_base_cpu_init(Object *obj)
> @@ -371,6 +396,9 @@ static void rv32_base_cpu_init(Object *obj)
>  register_cpu_props(obj);
>  /* Set latest version of privileged specification */
>  set_priv_version(env, PRIV_VERSION_1_12_0);
> +#ifndef CONFIG_USER_ONLY
> +set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
> +#endif
>  }
>
>  static void rv32_sifive_u_cpu_init(Object *obj)
> @@ -378,6 +406,9 @@ static void rv32_sifive_u_cpu_init(Object *obj)
>  CPURISCVState *env = &RISCV_CPU(obj)->env;
>  set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
>  set_priv_version(env, PRIV_VERSION_1_10_

Re: [PATCH v2] target/riscv: fix ctzw behavior

2023-02-05 Thread Alistair Francis
On Sat, Feb 4, 2023 at 6:25 PM Vladimir Isaev
 wrote:
>
> According to spec, ctzw should work with 32-bit register, not 64.
>
> For example, previous implementation returns 33 for (1<<33) input
> when the new one returns 32.
>
> Signed-off-by: Vladimir Isaev 
> Suggested-by: Richard Henderson 

Reviewed-by: Alistair Francis 

Alistair

> ---
> v2:
>- Use simpler solution suggested by Richard Henderson
> ---
>  target/riscv/insn_trans/trans_rvb.c.inc | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/insn_trans/trans_rvb.c.inc 
> b/target/riscv/insn_trans/trans_rvb.c.inc
> index e2b8329f1e5b..990bc94b9840 100644
> --- a/target/riscv/insn_trans/trans_rvb.c.inc
> +++ b/target/riscv/insn_trans/trans_rvb.c.inc
> @@ -401,6 +401,7 @@ static bool trans_ctzw(DisasContext *ctx, arg_ctzw *a)
>  {
>  REQUIRE_64BIT(ctx);
>  REQUIRE_ZBB(ctx);
> +ctx->ol = MXL_RV32;
>  return gen_unary(ctx, a, EXT_ZERO, gen_ctzw);
>  }
>
> --
> 2.39.1
>
>



Re: [PATCH] target/riscv: fix SBI getchar handler for KVM

2023-02-05 Thread Alistair Francis
On Sat, Feb 4, 2023 at 12:03 AM Vladimir Isaev
 wrote:
>
> Character must be returned via ret[0] field (copied to a0 by KVM).
>
> Return value should be set to 0 to indicate successful processing.
>
> Signed-off-by: Vladimir Isaev 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/kvm.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 30f21453d69c..0f932a5b966e 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -467,10 +467,11 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct 
> kvm_run *run)
>  case SBI_EXT_0_1_CONSOLE_GETCHAR:
>  ret = qemu_chr_fe_read_all(serial_hd(0)->be, &ch, sizeof(ch));
>  if (ret == sizeof(ch)) {
> -run->riscv_sbi.args[0] = ch;
> +run->riscv_sbi.ret[0] = ch;
>  } else {
> -run->riscv_sbi.args[0] = -1;
> +run->riscv_sbi.ret[0] = -1;
>  }
> +ret = 0;
>  break;
>  default:
>  qemu_log_mask(LOG_UNIMP,
> --
> 2.39.1
>
>



Re: [PATCH v5 01/14] RISC-V: Adding XTheadCmo ISA extension

2023-02-05 Thread Alistair Francis
On Wed, Feb 1, 2023 at 6:21 AM Christoph Muellner
 wrote:
>
> From: Christoph Müllner 
>
> This patch adds support for the XTheadCmo ISA extension.
> To avoid interfering with standard extensions, decoder and translation
> are in its own xthead* specific files.
> Future patches should be able to easily add additional T-Head extension.
>
> The implementation does not have much functionality (besides accepting
> the instructions and not qualifying them as illegal instructions if
> the hart executes in the required privilege level for the instruction),
> as QEMU does not model CPU caches and instructions are documented
> to not raise any exceptions.
>
> Co-developed-by: LIU Zhiwei 
> Signed-off-by: Christoph Müllner 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> Changes in v2:
> - Add ISA_EXT_DATA_ENTRY()
> - Explicit test for PRV_U
> - Encapsule access to env-priv in inline function
> - Use single decoder for XThead extensions
>
> Changes in v3:
> - Appling mask TB_FLAGS_PRIV_MMU_MASK to use of ctx->mem_idx
> - Removing code from test macro REQUIRE_PRIV_MSU()
> - Removing PRV_H from test macro REQUIRE_PRIV_MS()
> - Remove unrelated clean-up
> - Reorder decoder includes
>
> Changes in v4:
> - Reorder decoder includes
>
>  target/riscv/cpu.c |  2 +
>  target/riscv/cpu.h |  1 +
>  target/riscv/insn_trans/trans_xthead.c.inc | 81 ++
>  target/riscv/meson.build   |  1 +
>  target/riscv/translate.c   |  8 +++
>  target/riscv/xthead.decode | 38 ++
>  6 files changed, 131 insertions(+)
>  create mode 100644 target/riscv/insn_trans/trans_xthead.c.inc
>  create mode 100644 target/riscv/xthead.decode
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 14a7027095..6ea61e5b22 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -109,6 +109,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
>  ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0, ext_svinval),
>  ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
>  ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
> +ISA_EXT_DATA_ENTRY(xtheadcmo, true, PRIV_VERSION_1_11_0, ext_xtheadcmo),
>  ISA_EXT_DATA_ENTRY(xventanacondops, true, PRIV_VERSION_1_12_0, 
> ext_XVentanaCondOps),
>  };
>
> @@ -1088,6 +1089,7 @@ static Property riscv_cpu_extensions[] = {
>  DEFINE_PROP_BOOL("zmmul", RISCVCPU, cfg.ext_zmmul, false),
>
>  /* Vendor-specific custom extensions */
> +DEFINE_PROP_BOOL("xtheadcmo", RISCVCPU, cfg.ext_xtheadcmo, false),
>  DEFINE_PROP_BOOL("xventanacondops", RISCVCPU, cfg.ext_XVentanaCondOps, 
> false),
>
>  /* These are experimental so mark with 'x-' */
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index bcf0826753..d3ebc6f112 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -473,6 +473,7 @@ struct RISCVCPUConfig {
>  uint64_t mimpid;
>
>  /* Vendor-specific custom extensions */
> +bool ext_xtheadcmo;
>  bool ext_XVentanaCondOps;
>
>  uint8_t pmu_num;
> diff --git a/target/riscv/insn_trans/trans_xthead.c.inc 
> b/target/riscv/insn_trans/trans_xthead.c.inc
> new file mode 100644
> index 00..24acaf188c
> --- /dev/null
> +++ b/target/riscv/insn_trans/trans_xthead.c.inc
> @@ -0,0 +1,81 @@
> +/*
> + * RISC-V translation routines for the T-Head vendor extensions (xthead*).
> + *
> + * Copyright (c) 2022 VRULL GmbH.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + */
> +
> +#define REQUIRE_XTHEADCMO(ctx) do {  \
> +if (!ctx->cfg_ptr->ext_xtheadcmo) {  \
> +return false;\
> +}\
> +} while (0)
> +
> +/* XTheadCmo */
> +
> +static inline int priv_level(DisasContext *ctx)
> +{
> +#ifdef CONFIG_USER_ONLY
> +return PRV_U;
> +#else
> + /* Priv level is part of mem_idx. */
> +return ctx->mem_idx & TB_FLAGS_PRIV_MMU_MASK;
> +#endif
> +}
> +
> +/* Test if priv level is M, S, or U (cannot fail). */
> +#define REQUIRE_PRIV_MSU(ctx)
> +
> +/* Test if priv level is M or S. */
> +#define REQUIRE_PRIV_MS(ctx)\
> +do {\
> +int priv = priv_level(ctx); \
> +  

Re: [PATCH: fix for virt instr exception] target/riscv: fix for virtual instr exception

2023-02-05 Thread Alistair Francis
On Sat, Jan 28, 2023 at 6:36 AM Deepak Gupta  wrote:
>
> commit fb3f3730e4 added mechanism to generate virtual instruction
> exception during instruction decode when virt is enabled.
>
> However in some situations, illegal instruction exception can be raised
> due to state of CPU. One such situation is implementing branch tracking.
> [1] An indirect branch if doesn't land on a landing pad instruction, then
> cpu must raise an illegal instruction exception.
> Implementation would raise such expcetion due to missing landing pad inst
> and not due to decode. Thus DisasContext must have `virt_inst_excp`
> initialized to false during DisasContxt initialization for TB.
>
> [1] - https://github.com/riscv/riscv-cfi
>
> Signed-off-by: Deepak Gupta 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/translate.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index df38db7553..76f61a39d3 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1167,6 +1167,7 @@ static void 
> riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>  ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED);
>  ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
>  ctx->zero = tcg_constant_tl(0);
> +ctx->virt_inst_excp = false;
>  }
>
>  static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
> --
> 2.25.1
>
>



Re: [PATCH 00/14] linux-user/sparc: Handle missing traps

2023-02-05 Thread Mark Cave-Ayland

On 02/02/2023 00:51, Richard Henderson wrote:


Lots of missing trap code for cpu_loop().

r~

Richard Henderson (14):
   linux-user/sparc: Raise SIGILL for all unhandled software traps
   linux-user/sparc: Tidy syscall trap
   linux-user/sparc: Use TT_TRAP for flush windows
   linux-user/sparc: Tidy window spill/fill traps
   linux-user/sparc: Fix sparc64_{get,set}_context traps
   linux-user/sparc: Handle software breakpoint trap
   linux-user/sparc: Handle division by zero traps
   linux-user/sparc: Handle getcc, setcc, getpsr traps
   linux-user/sparc: Handle priviledged opcode trap
   linux-user/sparc: Handle privilidged action trap


Minor spelling nit: s/priviledged/privileged/


   linux-user/sparc: Handle coprocessor disabled trap
   linux-user/sparc: Handle unimplemented flush trap
   linux-user/sparc: Handle floating-point exceptions
   linux-user/sparc: Handle tag overflow traps

  linux-user/sparc/target_signal.h |   2 +-
  linux-user/syscall_defs.h|   5 +
  target/sparc/cpu.h   |   3 +-
  linux-user/sparc/cpu_loop.c  | 170 +--
  linux-user/sparc/signal.c|  36 +++
  5 files changed, 167 insertions(+), 49 deletions(-)


Alas I'm not overly familiar with the Linux syscall implementation on SPARC (all I 
can really do is run a chroot debian ports install for testing), however if all your 
local tests pass then I'm happy for this to go via the tcg or linux-user trees.



ATB,

Mark.



Re: [PATCH 0/9] hw: Use QOM alias properties and few QOM/QDev cleanups

2023-02-05 Thread Mark Cave-Ayland

On 03/02/2023 11:36, Philippe Mathieu-Daudé wrote:


These patches are extracted from a QOM/QDev refactor series,
so they are preliminary cleanups noticed while working on it:

- Use correct type when calling qdev_prop_set_xxx()
- Unify some qdev properties in MIPS models
- Replace intermediate properties by link properties
- Remove DEFINE_PROP_DMAADDR() macro which is used one time
- Use qdev_realize_and_unref() instead of open-coding it

Philippe Mathieu-Daudé (9):
   hw/i386/sgx: Do not open-code qdev_realize_and_unref()
   hw/ppc/sam460ex: Correctly set MAL properties
   hw/arm/nrf51: QOM-alias 'flash-size' property in SoC object
   hw/arm/fsl-imx: QOM-alias 'phy-num' property in SoC object
   hw/usb/hcd-ohci: Include missing 'sysbus.h' header
   hw/display/sm501: QOM-alias 'dma-offset' property in chipset object
   hw/qdev: Remove DEFINE_PROP_DMAADDR() and 'hw/qdev-dma.h'
   hw/mips: Declare all length properties as unsigned
   hw/mips/itu: Pass SAAR using QOM link property

  hw/arm/fsl-imx25.c   |  3 +--
  hw/arm/fsl-imx6.c|  3 +--
  hw/arm/fsl-imx6ul.c  |  8 
  hw/arm/fsl-imx7.c| 12 ++--
  hw/arm/microbit.c|  5 -
  hw/arm/nrf51_soc.c   | 10 +-
  hw/display/sm501.c   | 22 +++---
  hw/i386/sgx.c|  5 ++---
  hw/intc/mips_gic.c   |  4 ++--
  hw/mips/boston.c |  2 +-
  hw/mips/cps.c| 35 ---
  hw/mips/malta.c  |  2 +-
  hw/misc/mips_cmgcr.c |  2 +-
  hw/misc/mips_itu.c   | 30 --
  hw/nvram/nrf51_nvm.c |  6 +-
  hw/ppc/sam460ex.c|  4 ++--
  hw/sh4/r2d.c |  2 +-
  hw/usb/hcd-ohci-pci.c|  1 -
  hw/usb/hcd-ohci.c|  3 +--
  hw/usb/hcd-ohci.h|  1 +
  include/hw/arm/fsl-imx25.h   |  1 -
  include/hw/arm/fsl-imx6.h|  1 -
  include/hw/arm/fsl-imx6ul.h  |  2 --
  include/hw/arm/fsl-imx7.h|  1 -
  include/hw/arm/nrf51_soc.h   |  1 -
  include/hw/intc/mips_gic.h   |  4 ++--
  include/hw/misc/mips_cmgcr.h |  2 +-
  include/hw/misc/mips_itu.h   |  9 -
  include/hw/qdev-dma.h| 16 
  29 files changed, 84 insertions(+), 113 deletions(-)
  delete mode 100644 include/hw/qdev-dma.h


I must admit to being slightly nervous about using QOM alias properties in this way, 
simply because you start creating implicit dependencies between QOM objects. How 
would this work when trying to build machines from configuration files and/or the 
monitor? Or are the changes restricted to container devices i.e. those which consist 
of in-built child devices?



ATB,

Mark.



Re: [PATCH v2 0/2] mac_nvram: Add block backend to persist NVRAM contents

2023-02-05 Thread Mark Cave-Ayland

On 02/02/2023 00:24, BALATON Zoltan wrote:


Same as v1 just split in two patches as suggested by Mark.

Regards,
BALATON Zoltan

BALATON Zoltan (2):
   mac_nvram: Add block backend to persist NVRAM contents
   mac_oldworld: Allow specifying nvram backing store

  hw/nvram/mac_nvram.c | 28 
  hw/ppc/mac_oldworld.c|  8 +++-
  include/hw/nvram/mac_nvram.h |  1 +
  3 files changed, 36 insertions(+), 1 deletion(-)


This seems okay to me so:

Reviewed-by: Mark Cave-Ayland 

There was no further comment from Cédric regarding blk_check_size_and_read_all() so 
I've applied this to my qemu-macppc branch.



ATB,

Mark.



Re: [PATCH: fix for virt instr exception] target/riscv: fix for virtual instr exception

2023-02-05 Thread Alistair Francis
On Sat, Jan 28, 2023 at 6:36 AM Deepak Gupta  wrote:
>
> commit fb3f3730e4 added mechanism to generate virtual instruction
> exception during instruction decode when virt is enabled.
>
> However in some situations, illegal instruction exception can be raised
> due to state of CPU. One such situation is implementing branch tracking.
> [1] An indirect branch if doesn't land on a landing pad instruction, then
> cpu must raise an illegal instruction exception.
> Implementation would raise such expcetion due to missing landing pad inst
> and not due to decode. Thus DisasContext must have `virt_inst_excp`
> initialized to false during DisasContxt initialization for TB.
>
> [1] - https://github.com/riscv/riscv-cfi
>
> Signed-off-by: Deepak Gupta 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/translate.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index df38db7553..76f61a39d3 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1167,6 +1167,7 @@ static void 
> riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>  ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED);
>  ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER);
>  ctx->zero = tcg_constant_tl(0);
> +ctx->virt_inst_excp = false;
>  }
>
>  static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
> --
> 2.25.1
>
>



Re: [PATCH v2] target/riscv: fix ctzw behavior

2023-02-05 Thread Alistair Francis
On Sat, Feb 4, 2023 at 6:25 PM Vladimir Isaev
 wrote:
>
> According to spec, ctzw should work with 32-bit register, not 64.
>
> For example, previous implementation returns 33 for (1<<33) input
> when the new one returns 32.
>
> Signed-off-by: Vladimir Isaev 
> Suggested-by: Richard Henderson 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> v2:
>- Use simpler solution suggested by Richard Henderson
> ---
>  target/riscv/insn_trans/trans_rvb.c.inc | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/insn_trans/trans_rvb.c.inc 
> b/target/riscv/insn_trans/trans_rvb.c.inc
> index e2b8329f1e5b..990bc94b9840 100644
> --- a/target/riscv/insn_trans/trans_rvb.c.inc
> +++ b/target/riscv/insn_trans/trans_rvb.c.inc
> @@ -401,6 +401,7 @@ static bool trans_ctzw(DisasContext *ctx, arg_ctzw *a)
>  {
>  REQUIRE_64BIT(ctx);
>  REQUIRE_ZBB(ctx);
> +ctx->ol = MXL_RV32;
>  return gen_unary(ctx, a, EXT_ZERO, gen_ctzw);
>  }
>
> --
> 2.39.1
>
>



Re: [PATCH] target/riscv: fix SBI getchar handler for KVM

2023-02-05 Thread Alistair Francis
On Sat, Feb 4, 2023 at 12:03 AM Vladimir Isaev
 wrote:
>
> Character must be returned via ret[0] field (copied to a0 by KVM).
>
> Return value should be set to 0 to indicate successful processing.
>
> Signed-off-by: Vladimir Isaev 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/kvm.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 30f21453d69c..0f932a5b966e 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -467,10 +467,11 @@ static int kvm_riscv_handle_sbi(CPUState *cs, struct 
> kvm_run *run)
>  case SBI_EXT_0_1_CONSOLE_GETCHAR:
>  ret = qemu_chr_fe_read_all(serial_hd(0)->be, &ch, sizeof(ch));
>  if (ret == sizeof(ch)) {
> -run->riscv_sbi.args[0] = ch;
> +run->riscv_sbi.ret[0] = ch;
>  } else {
> -run->riscv_sbi.args[0] = -1;
> +run->riscv_sbi.ret[0] = -1;
>  }
> +ret = 0;
>  break;
>  default:
>  qemu_log_mask(LOG_UNIMP,
> --
> 2.39.1
>
>



Re: [PATCH v3 6/9] hw/i386/pc: Initialize ram_memory variable directly

2023-02-05 Thread Bernhard Beschow



Am 4. Februar 2023 15:26:13 UTC schrieb BALATON Zoltan :
>On Sat, 4 Feb 2023, Bernhard Beschow wrote:
>> Going through pc_memory_init() seems quite complicated for a simple
>> assignment.
>> 
>> Signed-off-by: Bernhard Beschow 
>> Reviewed-by: Philippe Mathieu-Daudé 
>> ---
>> include/hw/i386/pc.h | 1 -
>> hw/i386/pc.c | 2 --
>> hw/i386/pc_piix.c| 4 ++--
>> hw/i386/pc_q35.c | 5 ++---
>> 4 files changed, 4 insertions(+), 8 deletions(-)
>> 
>> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
>> index 66e3d059ef..b60b95921b 100644
>> --- a/include/hw/i386/pc.h
>> +++ b/include/hw/i386/pc.h
>> @@ -162,7 +162,6 @@ void xen_load_linux(PCMachineState *pcms);
>> void pc_memory_init(PCMachineState *pcms,
>> MemoryRegion *system_memory,
>> MemoryRegion *rom_memory,
>> -MemoryRegion **ram_memory,
>> uint64_t pci_hole64_size);
>> uint64_t pc_pci_hole64_start(void);
>> DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 6e592bd969..8898cc9961 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -936,7 +936,6 @@ static hwaddr pc_max_used_gpa(PCMachineState *pcms, 
>> uint64_t pci_hole64_size)
>> void pc_memory_init(PCMachineState *pcms,
>> MemoryRegion *system_memory,
>> MemoryRegion *rom_memory,
>> -MemoryRegion **ram_memory,
>> uint64_t pci_hole64_size)
>> {
>> int linux_boot, i;
>> @@ -994,7 +993,6 @@ void pc_memory_init(PCMachineState *pcms,
>>  * Split single memory region and use aliases to address portions of it,
>>  * done for backwards compatibility with older qemus.
>>  */
>> -*ram_memory = machine->ram;
>> ram_below_4g = g_malloc(sizeof(*ram_below_4g));
>> memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", 
>> machine->ram,
>>  0, x86ms->below_4g_mem_size);
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 5bde4533cc..00ba725656 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -143,6 +143,7 @@ static void pc_init1(MachineState *machine,
>> if (xen_enabled()) {
>> xen_hvm_init_pc(pcms, &ram_memory);
>> } else {
>> +ram_memory = machine->ram;
>
>Maybe you could just replace the few places it's used with machine->ram 
>directly and get rid of the local variable. There seems to be no advantage 
>storing it in a local just to use it once (in q35 below) or twice in pc-piix. 
>The local name is not even that much shorter so I don't see a reason to have 
>it in the first place,

Possible with q35 but not with piix which needs to get the RAM from Xen if 
running in this mode. I'll adjust q35 then.

Best regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> if (!pcms->max_ram_below_4g) {
>> pcms->max_ram_below_4g = 0xe000; /* default: 3.5G */
>> }
>> @@ -205,8 +206,7 @@ static void pc_init1(MachineState *machine,
>> 
>> /* allocate ram and load rom/bios */
>> if (!xen_enabled()) {
>> -pc_memory_init(pcms, system_memory,
>> -   rom_memory, &ram_memory, hole64_size);
>> +pc_memory_init(pcms, system_memory, rom_memory, hole64_size);
>> } else {
>> pc_system_flash_cleanup_unused(pcms);
>> if (machine->kernel_filename != NULL) {
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index 8253b49296..88f0981f50 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -129,7 +129,7 @@ static void pc_q35_init(MachineState *machine)
>> MemoryRegion *system_io = get_system_io();
>> MemoryRegion *pci_memory;
>> MemoryRegion *rom_memory;
>> -MemoryRegion *ram_memory;
>> +MemoryRegion *ram_memory = machine->ram;
>> GSIState *gsi_state;
>> ISABus *isa_bus;
>> int i;
>> @@ -216,8 +216,7 @@ static void pc_q35_init(MachineState *machine)
>> }
>> 
>> /* allocate ram and load rom/bios */
>> -pc_memory_init(pcms, system_memory, rom_memory, &ram_memory,
>> -   pci_hole64_size);
>> +pc_memory_init(pcms, system_memory, rom_memory, pci_hole64_size);
>> 
>> object_property_add_child(OBJECT(machine), "q35", OBJECT(q35_host));
>> object_property_set_link(OBJECT(q35_host), MCH_HOST_PROP_RAM_MEM,
>>



Re: [PATCH v3 2/9] hw/pci-host/q35: Inline sysbus_add_io()

2023-02-05 Thread Bernhard Beschow



Am 5. Februar 2023 11:12:26 UTC schrieb "Philippe Mathieu-Daudé" 
:
>On 4/2/23 16:10, Bernhard Beschow wrote:
>> sysbus_add_io() just wraps memory_region_add_subregion() while also
>> obscuring where the memory is attached. So use
>> memory_region_add_subregion() directly and attach it to the existing
>> memory region s->mch.address_space_io which is set as an alias to
>> get_system_io() by the q35 machine.
>> 
>> Signed-off-by: Bernhard Beschow 
>> Reviewed-by: Thomas Huth 
>> ---
>>   hw/pci-host/q35.c | 6 --
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
>> index 26390863d6..fa05844319 100644
>> --- a/hw/pci-host/q35.c
>> +++ b/hw/pci-host/q35.c
>> @@ -50,10 +50,12 @@ static void q35_host_realize(DeviceState *dev, Error 
>> **errp)
>>   Q35PCIHost *s = Q35_HOST_DEVICE(dev);
>>   SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>   -sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
>> +memory_region_add_subregion(s->mch.address_space_io,
>> +MCH_HOST_BRIDGE_CONFIG_ADDR, 
>> &pci->conf_mem);
>>   sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
>
>This makes me wonder why MCH_PCI_DEVICE doesn't use the bus I/O space
>via pci_address_space_io(). IOW, why the MR like is in MCH_PCI_DEVICE
>and not Q35_HOST_DEVICE?

I think I have follow-up patches in the pipeline moving the MemoryRegion 
pointers to the host device. Interestingly, these pointers are only used during 
the realize phase and just needlessly occupy memory during the rest of the 
device's lifetime...



Re: [PATCH v2 03/10] softmmu/ioport: Remove unused functions

2023-02-05 Thread Bernhard Beschow



Am 5. Februar 2023 21:37:01 UTC schrieb Mark Cave-Ayland 
:
>On 26/01/2023 21:17, Bernhard Beschow wrote:
>
>> Signed-off-by: Bernhard Beschow 
>> ---
>>   include/exec/ioport.h |  2 --
>>   softmmu/ioport.c  | 24 
>>   2 files changed, 26 deletions(-)
>> 
>> diff --git a/include/exec/ioport.h b/include/exec/ioport.h
>> index ec3e8e5942..1ef5aebba3 100644
>> --- a/include/exec/ioport.h
>> +++ b/include/exec/ioport.h
>> @@ -67,7 +67,5 @@ void portio_list_init(PortioList *piolist, Object *owner,
>> void *opaque, const char *name,
>> MemoryRegion *address_space_io, uint16_t start);
>>   void portio_list_set_flush_coalesced(PortioList *piolist);
>> -void portio_list_destroy(PortioList *piolist);
>> -void portio_list_del(PortioList *piolist);
>> #endif /* IOPORT_H */
>> diff --git a/softmmu/ioport.c b/softmmu/ioport.c
>> index c92e3cb27d..0a55d39196 100644
>> --- a/softmmu/ioport.c
>> +++ b/softmmu/ioport.c
>> @@ -118,19 +118,6 @@ void portio_list_set_flush_coalesced(PortioList 
>> *piolist)
>>   piolist->flush_coalesced_mmio = true;
>>   }
>>   -void portio_list_destroy(PortioList *piolist)
>> -{
>> -MemoryRegionPortioList *mrpio;
>> -unsigned i;
>> -
>> -for (i = 0; i < piolist->nr; ++i) {
>> -mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, 
>> mr);
>> -object_unparent(OBJECT(&mrpio->mr));
>> -g_free(mrpio);
>> -}
>> -g_free(piolist->regions);
>> -}
>> -
>>   static const MemoryRegionPortio *find_portio(MemoryRegionPortioList *mrpio,
>>uint64_t offset, unsigned 
>> size,
>>bool write)
>> @@ -280,14 +267,3 @@ void portio_list_init(PortioList *piolist, Object 
>> *owner,
>>   /* There will always be an open sub-list.  */
>>   portio_list_add_1(piolist, pio_start, count, start, off_low, off_high);
>>   }
>> -
>> -void portio_list_del(PortioList *piolist)
>> -{
>> -MemoryRegionPortioList *mrpio;
>> -unsigned i;
>> -
>> -for (i = 0; i < piolist->nr; ++i) {
>> -mrpio = container_of(piolist->regions[i], MemoryRegionPortioList, 
>> mr);
>> -memory_region_del_subregion(piolist->address_space, &mrpio->mr);
>> -}
>> -}
>
>I think it may be worth leaving these functions. There were previous 
>discussions around the cmd646 and via PCI-IDE interfaces which have a bit in 
>PCI configuration space that switches the chip between compatibility (ISA) 
>mode and PCI mode. I could see that switching the device to PCI mode would 
>require removal of the old ISA ports, for example, as in PCI mode the 
>registers would be accessed exclusively via the PCI BAR.

Sure, I can skip this patch.

BR,
Bernhard

>
>
>ATB,
>
>Mark.



PING: [PATCH v4 00/12] Refactor cryptodev

2023-02-05 Thread zhenwei pi

Hi Michael

Please correct me if I miss anything...

On 1/29/23 10:57, zhenwei pi wrote:

v4 -> v5:
- suggested by MST, use 'PRIu32' instead of '%u' to print a uint32_t value
- correct *QCryptodevBackendClient* and *QCryptodevInfo* in qapi/cryptodev.json

v3 -> v4:
- a small change in 
'0005-cryptodev-Introduce-query-cryptodev-QMP-command.patch':
   use 'uint32' instead of 'int' to describe CryptodevBackendClient:queue
- fix compling warning(gcc)/error(clang-11) on 32 bit platform in
   '0007-hmp-add-cryptodev-info-command.patch':
   use 'printf("%u", client->queue)' instead of 'printf("%ld", client->queue)'

v2 -> v3:
- rebase code against the lastest commist: fb7e7990342e59cf67d
- document the missing fields in qapi/cryptodev.json
- rework statistics part: use 'query-stats' command instead of
   'query-cryptodev'(cryptodev: Support query-stats QMP command)

v1 -> v2:
- fix coding style and use 'g_strjoin()' instead of 'char services[128]'
(suggested by Dr. David Alan Gilbert)
- wrapper function 'cryptodev_backend_account' to record statistics, and
allocate sym_stat/asym_stat in cryptodev base class. see patch:
'cryptodev: Support statistics'.
- add more arguments into struct CryptoDevBackendOpInfo, then
cryptodev_backend_crypto_operation() uses *op_info only.
- support cryptodev QoS settings(BPS&OPS), both QEMU command line and QMP
command works fine.
- add myself as the maintainer for cryptodev.

v1:
- introduce cryptodev.json to describe the attributes of crypto device, then
drop duplicated type declare, remove some virtio related dependence.
- add statistics: OPS and bandwidth.
- add QMP command: query-cryptodev
- add HMP info command: cryptodev
- misc fix: detect akcipher capability instead of exposing akcipher service
unconditionally.

Zhenwei Pi (12):
   cryptodev: Introduce cryptodev.json
   cryptodev: Remove 'name' & 'model' fields
   cryptodev: Introduce cryptodev alg type in QAPI
   cryptodev: Introduce server type in QAPI
   cryptodev: Introduce 'query-cryptodev' QMP command
   cryptodev-builtin: Detect akcipher capability
   hmp: add cryptodev info command
   cryptodev: Use CryptoDevBackendOpInfo for operation
   cryptodev: Account statistics
   cryptodev: support QoS
   cryptodev: Support query-stats QMP command
   MAINTAINERS: add myself as the maintainer for cryptodev

  MAINTAINERS |   2 +
  backends/cryptodev-builtin.c|  42 ++--
  backends/cryptodev-lkcf.c   |  19 +-
  backends/cryptodev-vhost-user.c |  13 +-
  backends/cryptodev-vhost.c  |   4 +-
  backends/cryptodev.c| 419 ++--
  hmp-commands-info.hx|  14 ++
  hw/virtio/virtio-crypto.c   |  48 +++-
  include/monitor/hmp.h   |   1 +
  include/sysemu/cryptodev.h  |  95 
  monitor/hmp-cmds.c  |  42 
  monitor/qmp-cmds.c  |   2 +
  qapi/cryptodev.json | 143 +++
  qapi/meson.build|   1 +
  qapi/qapi-schema.json   |   1 +
  qapi/qom.json   |   8 +-
  qapi/stats.json |  10 +-
  17 files changed, 744 insertions(+), 120 deletions(-)
  create mode 100644 qapi/cryptodev.json



--
zhenwei pi



Re: [PATCH 3/3] hw/mips: Add MIPS virt board

2023-02-05 Thread Jiaxun Yang



> 2023年2月5日 11:48,Philippe Mathieu-Daudé  写道:
> 
> Hi Jiaxun,
> 
> On 2/2/23 14:21, Jiaxun Yang wrote:
>> MIPS virt board is design to utilize existing VirtIO infrastures
>> but also comptitable with MIPS's existing internal simulation tools.
>> It includes virtio-mmio, pcie gpex, flash rom, fw_cfg, goldfish-rtc,
>> and optional goldfish_pic in case MIPS GIC is not present.
> 
> Is it worth using the CPS/GIC? Can't we using the goldfish PIC
> regardless CPS availability? Did you run performance comparison?

goldfish_pic don’t have IPI infra so we must reinvent another SMP mechanism :-(

The interrupt performance should be close as the interrupt handling flow is 
almost
the same.

Also it can help us prepare for I6400 vGIC support.

Thanks.
- Jiaxun


> 
>> It should be able to cooperate with any MIPS CPU cores.
>> Signed-off-by: Jiaxun Yang 
>> ---
>> v1:
>>  - Rename to virt board
>>  - Convert BIOS flash to ROM
>>  - Cleanups
>> ---
>>  MAINTAINERS |7 +
>>  configs/devices/mips-softmmu/common.mak |1 +
>>  docs/system/target-mips.rst |   24 +
>>  hw/mips/Kconfig |   18 +
>>  hw/mips/meson.build |1 +
>>  hw/mips/virt.c  | 1015 +++
>>  6 files changed, 1066 insertions(+)
>>  create mode 100644 hw/mips/virt.c
> 




[PATCH v2 2/2] virtio_net: just purge tx when dev/queue reset

2023-02-05 Thread Xuan Zhuo
When dev/queue reset, we should just purge all packet, not try to flush
the async packets. When flush these async packets, the
callback(virtio_net_tx_complete) will try to flush new packets from tx
queue.

Fixes: 7dc6be52 ("virtio-net: support queue reset")
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1451
Reported-by: Alexander Bulekov 
Signed-off-by: Xuan Zhuo 
---
 hw/net/virtio-net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 6daa1e5ac1..2ac6d3dad9 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -570,7 +570,7 @@ static void virtio_net_queue_reset(VirtIODevice *vdev, 
uint32_t queue_index)
 vhost_net_virtqueue_reset(vdev, nc, queue_index);
 }
 
-flush_or_purge_queued_packets(nc);
+qemu_purge_queued_packets(nc);
 }
 
 static void virtio_net_queue_enable(VirtIODevice *vdev, uint32_t queue_index)
-- 
2.32.0.3.g01195cf9f




[PATCH v2 1/2] virtio_net: virtio_net_tx_complete() stop flush new packets for purge operation

2023-02-05 Thread Xuan Zhuo
For async tx, virtio_net_tx_complete() is called when purge or flush
operation is done. But for purge operation, we should not try to flush
new packet from tx queue. The purge operation means we will stop the
queue soon.

Signed-off-by: Xuan Zhuo 
---
 hw/net/virtio-net.c | 32 ++--
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3ae909041a..6daa1e5ac1 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2601,21 +2601,25 @@ static void virtio_net_tx_complete(NetClientState *nc, 
ssize_t len)
 q->async_tx.elem = NULL;
 
 virtio_queue_set_notification(q->tx_vq, 1);
-ret = virtio_net_flush_tx(q);
-if (ret >= n->tx_burst) {
-/*
- * the flush has been stopped by tx_burst
- * we will not receive notification for the
- * remainining part, so re-schedule
- */
-virtio_queue_set_notification(q->tx_vq, 0);
-if (q->tx_bh) {
-qemu_bh_schedule(q->tx_bh);
-} else {
-timer_mod(q->tx_timer,
-  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + n->tx_timeout);
+
+/* len == 0 means purge, we should not flush new tx packets. */
+if (len) {
+ret = virtio_net_flush_tx(q);
+if (ret >= n->tx_burst) {
+/*
+ * the flush has been stopped by tx_burst
+ * we will not receive notification for the
+ * remainining part, so re-schedule
+ */
+virtio_queue_set_notification(q->tx_vq, 0);
+if (q->tx_bh) {
+qemu_bh_schedule(q->tx_bh);
+} else {
+timer_mod(q->tx_timer,
+  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 
n->tx_timeout);
+}
+q->tx_waiting = 1;
 }
-q->tx_waiting = 1;
 }
 }
 
-- 
2.32.0.3.g01195cf9f




  1   2   >