Re: [PATCH 07/10] tcg: implement bulletproof JIT
On Mon, 12 Oct 2020, Joelle van Dyne wrote:
From: osy
On iOS, we cannot allocate RWX pages without special entitlements. As a
workaround, we can a RX region and then mirror map it to a separate RX
Missing a verb here: "we can a RX region"
region. Then we can write to one region and execute from the other one.
To better keep track of pointers to RW/RX memory, we mark any tcg_insn_unit
pointers as `const` if they will never be written to. We also define a new
macro `TCG_CODE_PTR_RW` that returns a pointer to RW memory. Only the
difference between the two regions is stored in the TCG context.
Maybe it's easier to review if constification is split off as separate
patch before other changes.
To ensure cache coherency, we flush the data cache in the RW mapping and
then invalidate the instruction cache in the RX mapping (where applicable).
Because data cache flush is OS defined on some architectures, we do not
provide implementations for non iOS platforms (ARM/x86).
Signed-off-by: Joelle van Dyne
---
accel/tcg/cpu-exec.c | 7 +++-
accel/tcg/translate-all.c| 78 ++--
configure| 1 +
docs/devel/ios.rst | 40 ++
include/exec/exec-all.h | 8
include/tcg/tcg.h| 18 +++--
tcg/aarch64/tcg-target.c.inc | 48 +-
tcg/aarch64/tcg-target.h | 13 +-
tcg/arm/tcg-target.c.inc | 33 ---
tcg/arm/tcg-target.h | 9 -
tcg/i386/tcg-target.c.inc| 28 ++---
tcg/i386/tcg-target.h| 24 ++-
tcg/mips/tcg-target.c.inc| 64 +
tcg/mips/tcg-target.h| 8 +++-
tcg/ppc/tcg-target.c.inc | 55 -
tcg/ppc/tcg-target.h | 8 +++-
tcg/riscv/tcg-target.c.inc | 51 +--
tcg/riscv/tcg-target.h | 9 -
tcg/s390/tcg-target.c.inc| 25 ++--
tcg/s390/tcg-target.h| 13 +-
tcg/sparc/tcg-target.c.inc | 33 +--
tcg/sparc/tcg-target.h | 8 +++-
tcg/tcg-ldst.c.inc | 2 +-
tcg/tcg-pool.c.inc | 9 +++--
tcg/tcg.c| 60 +--
tcg/tci/tcg-target.c.inc | 8 ++--
tcg/tci/tcg-target.h | 9 -
27 files changed, 481 insertions(+), 188 deletions(-)
create mode 100644 docs/devel/ios.rst
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 58aea605d8..821aefdea2 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -354,7 +354,12 @@ void tb_set_jmp_target(TranslationBlock *tb, int n,
uintptr_t addr)
if (TCG_TARGET_HAS_direct_jump) {
uintptr_t offset = tb->jmp_target_arg[n];
uintptr_t tc_ptr = (uintptr_t)tb->tc.ptr;
-tb_target_set_jmp_target(tc_ptr, tc_ptr + offset, addr);
+#if defined(CONFIG_IOS_JIT)
+uintptr_t wr_addr = tc_ptr + offset + tb->code_rw_mirror_diff;
+#else
+uintptr_t wr_addr = tc_ptr + offset;
+#endif
+tb_target_set_jmp_target(tc_ptr, tc_ptr + offset, addr, wr_addr);
} else {
tb->jmp_target_arg[n] = addr;
}
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index d76097296d..76d8dc3d7b 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -60,6 +60,22 @@
#include "sysemu/cpu-timers.h"
#include "sysemu/tcg.h"
+#if defined(CONFIG_IOS_JIT)
+#include
+extern kern_return_t mach_vm_remap(vm_map_t target_task,
+ mach_vm_address_t *target_address,
+ mach_vm_size_t size,
+ mach_vm_offset_t mask,
+ int flags,
+ vm_map_t src_task,
+ mach_vm_address_t src_address,
+ boolean_t copy,
+ vm_prot_t *cur_protection,
+ vm_prot_t *max_protection,
+ vm_inherit_t inheritance
+ );
+#endif
+
/* #define DEBUG_TB_INVALIDATE */
/* #define DEBUG_TB_FLUSH */
/* make various TB consistency checks */
@@ -302,10 +318,13 @@ static target_long decode_sleb128(uint8_t **pp)
static int encode_search(TranslationBlock *tb, uint8_t *block)
{
-uint8_t *highwater = tcg_ctx->code_gen_highwater;
-uint8_t *p = block;
+uint8_t *highwater;
+uint8_t *p;
int i, j, n;
+highwater = (uint8_t *)TCG_CODE_PTR_RW(tcg_ctx,
+ tcg_ctx->code_gen_highwater);
+p = (uint8_t *)TCG_CODE_PTR_RW(tcg_ctx, block);
Why do you need explicit casts here? Can this be avoided by using
appropriate type or within the macro (I haven't checked this at all just
dislike casts as they can hide problems otherwise caught by the compiler).
Regards,
BALATON Zoltan
for (i = 0, n = tb->icount; i < n; ++i) {
target_ulong prev;
@@ -329,7 +348,7 @@
Re: [PATCH 06/10] coroutine: add libucontext as external library
On Tue, 13 Oct 2020, Stefan Hajnoczi wrote: On Mon, Oct 12, 2020 at 04:29:35PM -0700, Joelle van Dyne wrote: From: osy iOS does not support ucontext natively for aarch64 and the sigaltstack is also unsupported (even worse, it fails silently, see: https://openradar.appspot.com/13002712 ) As a workaround we include a library implementation of ucontext and add it as a build option. Signed-off-by: Joelle van Dyne Hi, Thanks for sending posting this! Please indicate what license libucontext is under, that it is compatible with QEMU's overall GPL v2 license, and update QEMU license https://github.com/utmapp/libucontext/blob/master/LICENSE Maybe the submodule repo should be mirrored in qemu.git eventually. documentation (LICENSE, etc), if necessary. Please update .gitlab-ci.yml with build tests. Is there a way to test building QEMU for iOS? If not, then it's difficult for the upstream QEMU project to carry iOS-specific features since we cannot test them. Build testing should be possible on OS X host that I think we already have provided it has the right XCode version installed. (Running it is difficult due to app deployment requirements of iOS devices.) But I don't know much about these, just trying to point at some possible directions to solve this. Regards, BALATON Zoltan
Re: [PATCH 04/10] meson: option to build as shared library
On Tue, 13 Oct 2020, Daniel P. Berrangé wrote: On Mon, Oct 12, 2020 at 04:29:33PM -0700, Joelle van Dyne wrote: From: osy On iOS, we cannot fork() new processes, so the best way to load QEMU into an app is through a shared library. We add a new configure option `--enable-shared-lib` that will build the bulk of QEMU into a shared lib. The usual executables will then link to the library. Note that QEMU as a combined work is licensed GPLv2-only, so if an app is linking to it as a shared library, the application's license has to be GPLv2 compatible. I fear that shipping as a shared library is an easy way for apps to get into a license violating situation without realizing. Please don't let that be an obstacle in merging this series. They'll do it anyway as seen here so having it upstream is probably better than having a lot of different/divergent forks. In case of UTM it seems to be licensed under Apache License 2.0: https://github.com/utmapp/UTM/blob/master/LICENSE which FSF says not compatible with GPLv2 but it is with GPLv3: http://www.gnu.org/licenses/license-list.html#apache2 Not sure however if that's for using Apache licenced part in GPLv2 code or the other way around like in UTM in which case I think the whole work will effectively become GPLv3 as most parts of QEMU is probably GPLv2+ already or BSD like free that should be possible to combine with only files explicitely GPLv2 in QEMU remaining at that license and UTM parts are Apache 2.0 when separated from QEMU. I have no idea about legal stuff whatsoever but combining two free software components should be legal some way (otherwise it's not possible to combine GPLv2 with GPLv3 either). I hope this does not turn into a legal bike shedding thread that results in finding out there are other than technical problems in the way to accept this series but the contrary, I'd say let the legal problems be handled by those using QEMU and don't reject patches based on maybe possible legal problems. Stating the licence clearly and maybe providing links to further resources to resolve licence problems in QEMU docs should be enough. Regards, BALATON Zoltan
Re: [PATCH v2 3/3] uninorth: use qdev gpios for PCI IRQs
On Tue, 13 Oct 2020, Mark Cave-Ayland wrote:
Currently an object link property is used to pass a reference to the OpenPIC
into the PCI host bridge so that pci_unin_init_irqs() can connect the PCI
IRQs to the PIC itself.
This can be simplified by defining the PCI IRQs as qdev gpios and then wiring
up the PCI IRQs to the PIC in the New World machine init function.
Signed-off-by: Mark Cave-Ayland
---
hw/pci-host/uninorth.c | 45 +++---
hw/ppc/mac_newworld.c | 24 --
include/hw/pci-host/uninorth.h | 2 --
3 files changed, 25 insertions(+), 46 deletions(-)
diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c
index 1ed1072eeb..0c0a9ecee1 100644
--- a/hw/pci-host/uninorth.c
+++ b/hw/pci-host/uninorth.c
@@ -32,8 +32,6 @@
#include "hw/pci-host/uninorth.h"
#include "trace.h"
-static const int unin_irq_line[] = { 0x1b, 0x1c, 0x1d, 0x1e };
-
static int pci_unin_map_irq(PCIDevice *pci_dev, int irq_num)
{
return (irq_num + (pci_dev->devfn >> 3)) & 3;
@@ -43,7 +41,7 @@ static void pci_unin_set_irq(void *opaque, int irq_num, int
level)
{
UNINHostState *s = opaque;
-trace_unin_set_irq(unin_irq_line[irq_num], level);
+trace_unin_set_irq(irq_num, level);
qemu_set_irq(s->irqs[irq_num], level);
}
@@ -112,15 +110,6 @@ static const MemoryRegionOps unin_data_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
-static void pci_unin_init_irqs(UNINHostState *s)
-{
-int i;
-
-for (i = 0; i < ARRAY_SIZE(s->irqs); i++) {
-s->irqs[i] = qdev_get_gpio_in(DEVICE(s->pic), unin_irq_line[i]);
-}
-}
-
static char *pci_unin_main_ofw_unit_address(const SysBusDevice *dev)
{
UNINHostState *s = UNI_NORTH_PCI_HOST_BRIDGE(dev);
@@ -141,7 +130,6 @@ static void pci_unin_main_realize(DeviceState *dev, Error
**errp)
PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
pci_create_simple(h->bus, PCI_DEVFN(11, 0), "uni-north-pci");
-pci_unin_init_irqs(s);
/* DEC 21154 bridge */
#if 0
@@ -172,15 +160,12 @@ static void pci_unin_main_init(Object *obj)
"unin-pci-hole", >pci_mmio,
0x8000ULL, 0x1000ULL);
-object_property_add_link(obj, "pic", TYPE_OPENPIC,
- (Object **) >pic,
- qdev_prop_allow_set_link_before_realize,
- 0);
-
sysbus_init_mmio(sbd, >conf_mem);
sysbus_init_mmio(sbd, >data_mem);
sysbus_init_mmio(sbd, >pci_hole);
sysbus_init_mmio(sbd, >pci_io);
+
+qdev_init_gpio_out(DEVICE(obj), s->irqs, ARRAY_SIZE(s->irqs));
}
static void pci_u3_agp_realize(DeviceState *dev, Error **errp)
@@ -196,7 +181,6 @@ static void pci_u3_agp_realize(DeviceState *dev, Error
**errp)
PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
pci_create_simple(h->bus, PCI_DEVFN(11, 0), "u3-agp");
-pci_unin_init_irqs(s);
}
static void pci_u3_agp_init(Object *obj)
@@ -220,15 +204,12 @@ static void pci_u3_agp_init(Object *obj)
"unin-pci-hole", >pci_mmio,
0x8000ULL, 0x7000ULL);
-object_property_add_link(obj, "pic", TYPE_OPENPIC,
- (Object **) >pic,
- qdev_prop_allow_set_link_before_realize,
- 0);
-
sysbus_init_mmio(sbd, >conf_mem);
sysbus_init_mmio(sbd, >data_mem);
sysbus_init_mmio(sbd, >pci_hole);
sysbus_init_mmio(sbd, >pci_io);
+
+qdev_init_gpio_out(DEVICE(obj), s->irqs, ARRAY_SIZE(s->irqs));
}
static void pci_unin_agp_realize(DeviceState *dev, Error **errp)
@@ -244,7 +225,6 @@ static void pci_unin_agp_realize(DeviceState *dev, Error
**errp)
PCI_DEVFN(11, 0), 4, TYPE_PCI_BUS);
pci_create_simple(h->bus, PCI_DEVFN(11, 0), "uni-north-agp");
-pci_unin_init_irqs(s);
}
static void pci_unin_agp_init(Object *obj)
@@ -259,13 +239,10 @@ static void pci_unin_agp_init(Object *obj)
memory_region_init_io(>data_mem, OBJECT(h), _host_data_le_ops,
obj, "unin-agp-conf-data", 0x1000);
-object_property_add_link(obj, "pic", TYPE_OPENPIC,
- (Object **) >pic,
- qdev_prop_allow_set_link_before_realize,
- 0);
-
sysbus_init_mmio(sbd, >conf_mem);
sysbus_init_mmio(sbd, >data_mem);
+
+qdev_init_gpio_out(DEVICE(obj), s->irqs, ARRAY_SIZE(s->irqs));
}
static void pci_unin_internal_realize(DeviceState *dev, Error **errp)
@@ -281,7 +258,6 @@ static void pci_unin_internal_realize(DeviceState *dev,
Error **errp)
PCI_DEVFN(14, 0), 4, TYPE_PCI_BUS);
pci_create_simple(h->bus, PCI_DEVFN(14, 0), "uni-north-internal-pci");
-pci_unin_init_irqs(s);
}
static void pci_unin_internal_init(Object *obj)
@@ -296,13 +272,10 @@ static void pci_unin_internal_init(Object *obj)
Re: [PATCH] Deprecate TileGX port
On Mon, 12 Oct 2020, Peter Maydell wrote:
Deprecate our TileGX target support:
* we have no active maintainer for it
* it has had essentially no contributions (other than tree-wide cleanups
and similar) since it was first added
* the Linux kernel dropped support in 2018, as has glibc
Note the deprecation in the manual, but don't try to print a warning
when QEMU runs -- printing unsuppressable messages is more obtrusive
for linux-user mode than it would be for system-emulation mode, and
it doesn't seem worth trying to invent a new suppressible-error
system for linux-user just for this.
Signed-off-by: Peter Maydell
---
We discussed dropping this target last year:
https://patchew.org/QEMU/20191012071210.13632-1-phi...@redhat.com/
and before that in 2018, when Chen told us he didn't have time
to work on tilegx any more:
https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg03955.html
Given that tilegx is no longer in upstream Linux I think it makes sense
to finally deprecate-and-drop our linux-user support for it.
docs/system/deprecated.rst | 11 +++
1 file changed, 11 insertions(+)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 3a255591c34..e9097e089bb 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -387,6 +387,17 @@ The above, converted to the current supported format::
json:{"file.driver":"rbd", "file.pool":"rbd", "file.image":"name"}
+linux-user mode CPUs
+
+
+``tilegx`` CPUs (since 5.1.0)
+'
Is that 5.2?
Regards,
BALATON Zoltan
+
+The ``tilegx`` guest CPU support (which was only implemented in
+linux-user mode) is deprecated and will be removed in a future version
+of QEMU. Support for this CPU was removed from the upstream Linux
+kernel in 2018, and has also been dropped from glibc.
+
Related binaries
Re: [PATCH 1/5] hw/pci-host/bonito: Make PCI_ADDR() macro more readable
On Mon, 12 Oct 2020, Philippe Mathieu-Daudé wrote: From: Philippe Mathieu-Daudé The PCI_ADDR() macro use generic PCI fields shifted by 8-bit. Rewrite it extracting the shift operation one layer. Signed-off-by: Philippe Mathieu-Daudé --- hw/pci-host/bonito.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c index a99eced0657..abb3ee86769 100644 --- a/hw/pci-host/bonito.c +++ b/hw/pci-host/bonito.c @@ -196,8 +196,8 @@ FIELD(BONGENCFG, PCIQUEUE, 12, 1) #define PCI_IDSEL_VIA686B (1 << PCI_IDSEL_VIA686B_BIT) #define PCI_ADDR(busno , devno , funno , regno) \ -busno) << 16) & 0xff) + (((devno) << 11) & 0xf800) + \ -(((funno) << 8) & 0x700) + (regno)) +busno) << 8) & 0xff00) + (((devno) << 3) & 0xf8) + \ +(((funno) & 0x7) << 8) + (regno)) Are you missing a << 8 somewhere before + (regno) or both of these are equally unreadable and I've missed something? This seems to be completely replaced by next patch so what's the point of this change? Regards, BALATON Zoltan typedef struct BonitoState BonitoState;
Re: [PATCH] hw/pci-host/grackle: Verify PIC link is properly set
On Mon, 12 Oct 2020, David Gibson wrote:
On Mon, Oct 12, 2020 at 08:21:41AM +0200, Philippe Mathieu-Daudé wrote:
On 10/12/20 12:34 AM, David Gibson wrote:
On Sun, Oct 11, 2020 at 09:03:32PM +0200, Philippe Mathieu-Daudé wrote:
The Grackle PCI host model expects the interrupt controller
being set, but does not verify it is present. Add a check to
help developers using this model.
I don't think thaqt's very likely, but, sure, applied to ppc-for-5.2
Do you want I correct the description as:
"The Grackle PCI host model expects the interrupt controller
being set, but does not verify it is present.
A developer basing its implementation on the Grackle model
might hit this problem. Add a check to help future developers
using this model as reference."?
No, it's fine. All I was saying is that the chances of anyone using
Grackle in future seem very low to me.
So maybe an assert instead of a user visible error would be enough?
Regards,
BALATON Zoltan
Signed-off-by: Philippe Mathieu-Daudé
---
hw/pci-host/grackle.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index 57c29b20afb..20361d215ca 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -76,6 +76,10 @@ static void grackle_realize(DeviceState *dev, Error **errp)
GrackleState *s = GRACKLE_PCI_HOST_BRIDGE(dev);
PCIHostState *phb = PCI_HOST_BRIDGE(dev);
+if (!s->pic) {
+error_setg(errp, TYPE_GRACKLE_PCI_HOST_BRIDGE ": 'pic' link not set");
+return;
+}
phb->bus = pci_register_root_bus(dev, NULL,
pci_grackle_set_irq,
pci_grackle_map_irq,
Re: [PATCH qemu v9] spapr: Implement Open Firmware client interface
On Mon, 12 Oct 2020, Alexey Kardashevskiy wrote: On 29/09/2020 20:35, Alexey Kardashevskiy wrote: On 16/07/2020 23:22, David Gibson wrote: On Thu, Jul 16, 2020 at 07:04:56PM +1000, Alexey Kardashevskiy wrote: Ping? I kinda realize it is not going to replace SLOF any time soon but still... Yeah, I know. I just haven't had time to consider it. Priority starvation. Still? :) Ping? +1, I'd like to see this merged and experiment with it to emulate firmware for pegasos2 but I'd rather use the final version than something off-tree which may end up different when gets upstream. Is there a way I could help with this? Regards, BALATON Zoltan
Re: Purpose of QOM properties registered at realize time?
On Tue, 6 Oct 2020, Eduardo Habkost wrote: Hi, While trying to understand how QOM properties are used in QEMU, I stumbled upon multiple cases where alias properties are added at realize time. Now, I don't understand why those properties exist. As the properties are added at realize time, I assume they aren't supposed to be touched by the user at all. If they are not supposed to be touched by the user, what exactly is the purpose of those QOM properties? For reference, these are the cases I've found: $ git grep -nwpE 'object_property_add_alias|qdev_pass_gpios|qdev_alias_all_properties' | grep -A1 realize hw/arm/allwinner-a10.c=71=static void aw_a10_realize(DeviceState *dev, Error **errp) hw/arm/allwinner-a10.c:89:qdev_pass_gpios(DEVICE(>intc), dev, NULL); -- hw/arm/allwinner-h3.c=232=static void allwinner_h3_realize(DeviceState *dev, Error **errp) hw/arm/allwinner-h3.c:359:object_property_add_alias(OBJECT(s), "sd-bus", OBJECT(>mmc0), -- hw/arm/armsse.c=429=static void armsse_realize(DeviceState *dev, Error **errp) hw/arm/armsse.c:1092:qdev_pass_gpios(dev_secctl, dev, "mscexp_status"); -- hw/arm/armv7m.c=149=static void armv7m_realize(DeviceState *dev, Error **errp) hw/arm/armv7m.c:219:qdev_pass_gpios(DEVICE(>nvic), dev, NULL); -- hw/arm/bcm2835_peripherals.c=128=static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp) hw/arm/bcm2835_peripherals.c:322:object_property_add_alias(OBJECT(s), "sd-bus", OBJECT(>gpio), "sd-bus"); -- hw/arm/bcm2836.c=69=static void bcm2836_realize(DeviceState *dev, Error **errp) hw/arm/bcm2836.c:87:object_property_add_alias(OBJECT(s), "sd-bus", OBJECT(>peripherals), hw/arm/msf2-soc.c=79=static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp) hw/arm/msf2-soc.c:170:object_property_add_alias(OBJECT(s), bus_name, -- hw/arm/nrf51_soc.c=58=static void nrf51_soc_realize(DeviceState *dev_soc, Error **errp) hw/arm/nrf51_soc.c:138:qdev_pass_gpios(DEVICE(>gpio), dev_soc, NULL); -- hw/arm/xlnx-zynqmp.c=276=static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) hw/arm/xlnx-zynqmp.c:522:object_property_add_alias(OBJECT(s), bus_name, sdhci, "sd-bus"); -- hw/misc/mac_via.c=1011=static void mac_via_realize(DeviceState *dev, Error **errp) hw/misc/mac_via.c:1028:object_property_add_alias(OBJECT(dev), "irq[0]", OBJECT(ms), Mark likely knows this one. I think he mentioned once that it may be used for storing reference to some object that needs to be accessed later but I could be wrong, haven't checked actual code only recalling from memory. Regards, BALATON Zoltan -- hw/ppc/spapr_drc.c=511=static void realize(DeviceState *d, Error **errp) hw/ppc/spapr_drc.c:530:object_property_add_alias(root_container, link_name, hw/riscv/sifive_e.c=186=static void sifive_e_soc_realize(DeviceState *dev, Error **errp) hw/riscv/sifive_e.c:233:qdev_pass_gpios(DEVICE(>gpio), dev, NULL); hw/riscv/sifive_u.c=651=static void sifive_u_soc_realize(DeviceState *dev, Error **errp) hw/riscv/sifive_u.c:743:qdev_pass_gpios(DEVICE(>gpio), dev, NULL);
Re: [PATCH 1/2] softmmu: move more files to softmmu/
On Tue, 6 Oct 2020, Paolo Bonzini wrote:
Keep most softmmu_ss files into the system-emulation-specific
directory.
The name of this dir may be misleading. I think it originally stood for
the actual MMU emulation but now it seems everything related to system
emulation is dumped here. Is it better to keep MMU emulation separate and
put other files in a "sysemu" dir or rename this dir if it keeps mixing
MMU emulation and system emulation parts? (I think the MMU emulation is a
weak part of QEMU regarding performance so it would be better to keep that
cleanly separated so that it's easier to analyse and optimise it in the
future, which is more difficult if it's mixed with other parts where it's
not even clear what constitutes the actual MMU emulation. So I vote for
keeping it separate.)
Regards,
BALATON Zoltan
Signed-off-by: Paolo Bonzini
---
meson.build | 10 --
bootdevice.c => softmmu/bootdevice.c | 0
device_tree.c => softmmu/device_tree.c | 0
dma-helpers.c => softmmu/dma-helpers.c | 0
softmmu/meson.build | 10 ++
qdev-monitor.c => softmmu/qdev-monitor.c | 0
qemu-seccomp.c => softmmu/qemu-seccomp.c | 0
tpm.c => softmmu/tpm.c | 0
8 files changed, 10 insertions(+), 10 deletions(-)
rename bootdevice.c => softmmu/bootdevice.c (100%)
rename device_tree.c => softmmu/device_tree.c (100%)
rename dma-helpers.c => softmmu/dma-helpers.c (100%)
rename qdev-monitor.c => softmmu/qdev-monitor.c (100%)
rename qemu-seccomp.c => softmmu/qemu-seccomp.c (100%)
rename tpm.c => softmmu/tpm.c (100%)
diff --git a/meson.build b/meson.build
index 17c89c87c6..0e0577e81e 100644
--- a/meson.build
+++ b/meson.build
@@ -1360,17 +1360,7 @@ blockdev_ss.add(files(
# os-win32.c does not
blockdev_ss.add(when: 'CONFIG_POSIX', if_true: files('os-posix.c'))
softmmu_ss.add(when: 'CONFIG_WIN32', if_true: [files('os-win32.c')])
-
softmmu_ss.add_all(blockdev_ss)
-softmmu_ss.add(files(
- 'bootdevice.c',
- 'dma-helpers.c',
- 'qdev-monitor.c',
-), sdl)
-
-softmmu_ss.add(when: 'CONFIG_TPM', if_true: files('tpm.c'))
-softmmu_ss.add(when: 'CONFIG_SECCOMP', if_true: [files('qemu-seccomp.c'),
seccomp])
-softmmu_ss.add(when: fdt, if_true: files('device_tree.c'))
common_ss.add(files('cpus-common.c'))
diff --git a/bootdevice.c b/softmmu/bootdevice.c
similarity index 100%
rename from bootdevice.c
rename to softmmu/bootdevice.c
diff --git a/device_tree.c b/softmmu/device_tree.c
similarity index 100%
rename from device_tree.c
rename to softmmu/device_tree.c
diff --git a/dma-helpers.c b/softmmu/dma-helpers.c
similarity index 100%
rename from dma-helpers.c
rename to softmmu/dma-helpers.c
diff --git a/softmmu/meson.build b/softmmu/meson.build
index 36c96e7b15..862ab24878 100644
--- a/softmmu/meson.build
+++ b/softmmu/meson.build
@@ -14,3 +14,13 @@ specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: [files(
specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TCG'], if_true: [files(
'icount.c'
)])
+
+softmmu_ss.add(files(
+ 'bootdevice.c',
+ 'dma-helpers.c',
+ 'qdev-monitor.c',
+), sdl)
+
+softmmu_ss.add(when: 'CONFIG_TPM', if_true: files('tpm.c'))
+softmmu_ss.add(when: 'CONFIG_SECCOMP', if_true: [files('qemu-seccomp.c'),
seccomp])
+softmmu_ss.add(when: fdt, if_true: files('device_tree.c'))
diff --git a/qdev-monitor.c b/softmmu/qdev-monitor.c
similarity index 100%
rename from qdev-monitor.c
rename to softmmu/qdev-monitor.c
diff --git a/qemu-seccomp.c b/softmmu/qemu-seccomp.c
similarity index 100%
rename from qemu-seccomp.c
rename to softmmu/qemu-seccomp.c
diff --git a/tpm.c b/softmmu/tpm.c
similarity index 100%
rename from tpm.c
rename to softmmu/tpm.c
Re: Use of "?" for help has been deprecated for 8 years, can we drop it?
On Thu, 1 Oct 2020, Eric Blake wrote: On 10/1/20 5:35 AM, Markus Armbruster wrote: We deprecated "?" more than eight years ago. We didn't have a deprecation process back then, but we did purge "?" from the documentation and from help texts. Can we finally drop it? I'm asking because there is a patch on the list that bypasses is_help_option() to not add deprecated "?" to a new place: "[PATCH v2 1/4] keyval: Parse help options". Did we ever issue a warning when it was used? It's easier to argue that it can be dropped if users had notice of some form or another. That said, I'm not heartbroken if we yank it immediately instead of letting it live for 2 more releases. How about keeping it as a convenience? I find it easier to type ? than help and often use it instead. It's sufficiently hidden to not cause any confusion for those who don't know and convenient for those who know. I agree with Peter that it should not be something that causes that much trouble that it would need to be removed immediately (or even in the future). Regrads, BALATON Zoltan
Re: [PATCH] load_elf: Remove unused address variables from callers
On Wed, 23 Sep 2020, BALATON Zoltan wrote:
On Tue, 7 Jul 2020, BALATON Zoltan wrote:
On Tue, 7 Jul 2020, Alistair Francis wrote:
On Sun, Jul 5, 2020 at 10:41 AM BALATON Zoltan wrote:
Several callers of load_elf() pass pointers for lowaddr and highaddr
parameters which are then not used for anything. This may stem from a
misunderstanding that load_elf need a value here but in fact it can
take NULL to ignore these values. Remove such unused variables and
pass NULL instead from callers that don't need these.
Signed-off-by: BALATON Zoltan
Reviewed-by: Alistair Francis
So this got a few review and acked by but since it touches multiple parts
who will actually send pull or merge it? I'd like to make sure this won't
miss the freeze deadline just because everybody thinks this should go in
via some other maintainer. What's the best way for this? Trivial
maintainers or Peter should handle such patches?
Ping? Could someone please queue this patch? It still seems to apply cleanly.
Forgot to include the link to the original patch:
http://patchwork.ozlabs.org/project/qemu-devel/patch/20200705174020.bdd01746...@zero.eik.bme.hu/
Regards,
BALATON Zoltan
---
hw/alpha/dp264.c | 8
hw/arm/armv7m.c| 4 +---
hw/cris/boot.c | 4 ++--
hw/microblaze/boot.c | 4 ++--
hw/mips/fuloong2e.c| 8
hw/moxie/moxiesim.c| 4 ++--
hw/nios2/boot.c| 4 ++--
hw/ppc/mac_newworld.c | 6 ++
hw/ppc/mac_oldworld.c | 6 ++
hw/ppc/ppc440_bamboo.c | 9 +++--
hw/ppc/sam460ex.c | 12 +---
hw/ppc/spapr.c | 11 ---
hw/ppc/virtex_ml507.c | 4 ++--
hw/riscv/boot.c| 8
hw/xtensa/sim.c| 3 +--
hw/xtensa/xtfpga.c | 3 +--
16 files changed, 41 insertions(+), 57 deletions(-)
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index f7751b18f6..4d24518d1d 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -62,8 +62,8 @@ static void clipper_init(MachineState *machine)
qemu_irq rtc_irq;
long size, i;
char *palcode_filename;
-uint64_t palcode_entry, palcode_low, palcode_high;
-uint64_t kernel_entry, kernel_low, kernel_high;
+uint64_t palcode_entry;
+uint64_t kernel_entry, kernel_low;
unsigned int smp_cpus = machine->smp.cpus;
/* Create up to 4 cpus. */
@@ -113,7 +113,7 @@ static void clipper_init(MachineState *machine)
exit(1);
}
size = load_elf(palcode_filename, NULL, cpu_alpha_superpage_to_phys,
-NULL, _entry, _low, _high,
NULL,
+NULL, _entry, NULL, NULL, NULL,
0, EM_ALPHA, 0, 0);
if (size < 0) {
error_report("could not load palcode '%s'", palcode_filename);
@@ -132,7 +132,7 @@ static void clipper_init(MachineState *machine)
uint64_t param_offset;
size = load_elf(kernel_filename, NULL,
cpu_alpha_superpage_to_phys,
-NULL, _entry, _low, _high,
NULL,
+NULL, _entry, _low, NULL, NULL,
0, EM_ALPHA, 0, 0);
if (size < 0) {
error_report("could not load kernel '%s'", kernel_filename);
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 3308211e9c..92f859d760 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -309,7 +309,6 @@ void armv7m_load_kernel(ARMCPU *cpu, const char
*kernel_filename, int mem_size)
{
int image_size;
uint64_t entry;
-uint64_t lowaddr;
int big_endian;
AddressSpace *as;
int asidx;
@@ -330,12 +329,11 @@ void armv7m_load_kernel(ARMCPU *cpu, const char
*kernel_filename, int mem_size)
if (kernel_filename) {
image_size = load_elf_as(kernel_filename, NULL, NULL, NULL,
- , , NULL,
+ , NULL, NULL,
NULL, big_endian, EM_ARM, 1, 0, as);
if (image_size < 0) {
image_size = load_image_targphys_as(kernel_filename, 0,
mem_size, as);
-lowaddr = 0;
}
if (image_size < 0) {
error_report("Could not load kernel '%s'", kernel_filename);
diff --git a/hw/cris/boot.c b/hw/cris/boot.c
index b8947bc660..aa8d2756d6 100644
--- a/hw/cris/boot.c
+++ b/hw/cris/boot.c
@@ -67,7 +67,7 @@ static uint64_t translate_kernel_address(void *opaque,
uint64_t addr)
void cris_load_image(CRISCPU *cpu, struct cris_load_info *li)
{
CPUCRISState *env = >env;
-uint64_t entry, high;
+uint64_t entry;
int kcmdline_len;
int image_size;
@@ -76,7 +76,7 @@ void cris_load_image(CRISCPU *cpu, struct
cris_load_info *li)
devboard SDK. */
image_size = load_elf(li->image_filename, NULL,
translate_kernel_address, NULL,
- , NULL, , NULL, 0, EM_CRIS, 0, 0);
+ , NULL, NULL, NULL, 0, EM_CRIS, 0, 0);
li->entry = entry;
Re: [PATCH] load_elf: Remove unused address variables from callers
On Tue, 7 Jul 2020, BALATON Zoltan wrote:
On Tue, 7 Jul 2020, Alistair Francis wrote:
On Sun, Jul 5, 2020 at 10:41 AM BALATON Zoltan wrote:
Several callers of load_elf() pass pointers for lowaddr and highaddr
parameters which are then not used for anything. This may stem from a
misunderstanding that load_elf need a value here but in fact it can
take NULL to ignore these values. Remove such unused variables and
pass NULL instead from callers that don't need these.
Signed-off-by: BALATON Zoltan
Reviewed-by: Alistair Francis
So this got a few review and acked by but since it touches multiple parts who
will actually send pull or merge it? I'd like to make sure this won't miss
the freeze deadline just because everybody thinks this should go in via some
other maintainer. What's the best way for this? Trivial maintainers or Peter
should handle such patches?
Ping? Could someone please queue this patch? It still seems to apply
cleanly.
Regards,
BALATON Zoltan
---
hw/alpha/dp264.c | 8
hw/arm/armv7m.c| 4 +---
hw/cris/boot.c | 4 ++--
hw/microblaze/boot.c | 4 ++--
hw/mips/fuloong2e.c| 8
hw/moxie/moxiesim.c| 4 ++--
hw/nios2/boot.c| 4 ++--
hw/ppc/mac_newworld.c | 6 ++
hw/ppc/mac_oldworld.c | 6 ++
hw/ppc/ppc440_bamboo.c | 9 +++--
hw/ppc/sam460ex.c | 12 +---
hw/ppc/spapr.c | 11 ---
hw/ppc/virtex_ml507.c | 4 ++--
hw/riscv/boot.c| 8
hw/xtensa/sim.c| 3 +--
hw/xtensa/xtfpga.c | 3 +--
16 files changed, 41 insertions(+), 57 deletions(-)
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index f7751b18f6..4d24518d1d 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -62,8 +62,8 @@ static void clipper_init(MachineState *machine)
qemu_irq rtc_irq;
long size, i;
char *palcode_filename;
-uint64_t palcode_entry, palcode_low, palcode_high;
-uint64_t kernel_entry, kernel_low, kernel_high;
+uint64_t palcode_entry;
+uint64_t kernel_entry, kernel_low;
unsigned int smp_cpus = machine->smp.cpus;
/* Create up to 4 cpus. */
@@ -113,7 +113,7 @@ static void clipper_init(MachineState *machine)
exit(1);
}
size = load_elf(palcode_filename, NULL, cpu_alpha_superpage_to_phys,
-NULL, _entry, _low, _high,
NULL,
+NULL, _entry, NULL, NULL, NULL,
0, EM_ALPHA, 0, 0);
if (size < 0) {
error_report("could not load palcode '%s'", palcode_filename);
@@ -132,7 +132,7 @@ static void clipper_init(MachineState *machine)
uint64_t param_offset;
size = load_elf(kernel_filename, NULL,
cpu_alpha_superpage_to_phys,
-NULL, _entry, _low, _high,
NULL,
+NULL, _entry, _low, NULL, NULL,
0, EM_ALPHA, 0, 0);
if (size < 0) {
error_report("could not load kernel '%s'", kernel_filename);
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 3308211e9c..92f859d760 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -309,7 +309,6 @@ void armv7m_load_kernel(ARMCPU *cpu, const char
*kernel_filename, int mem_size)
{
int image_size;
uint64_t entry;
-uint64_t lowaddr;
int big_endian;
AddressSpace *as;
int asidx;
@@ -330,12 +329,11 @@ void armv7m_load_kernel(ARMCPU *cpu, const char
*kernel_filename, int mem_size)
if (kernel_filename) {
image_size = load_elf_as(kernel_filename, NULL, NULL, NULL,
- , , NULL,
+ , NULL, NULL,
NULL, big_endian, EM_ARM, 1, 0, as);
if (image_size < 0) {
image_size = load_image_targphys_as(kernel_filename, 0,
mem_size, as);
-lowaddr = 0;
}
if (image_size < 0) {
error_report("Could not load kernel '%s'", kernel_filename);
diff --git a/hw/cris/boot.c b/hw/cris/boot.c
index b8947bc660..aa8d2756d6 100644
--- a/hw/cris/boot.c
+++ b/hw/cris/boot.c
@@ -67,7 +67,7 @@ static uint64_t translate_kernel_address(void *opaque,
uint64_t addr)
void cris_load_image(CRISCPU *cpu, struct cris_load_info *li)
{
CPUCRISState *env = >env;
-uint64_t entry, high;
+uint64_t entry;
int kcmdline_len;
int image_size;
@@ -76,7 +76,7 @@ void cris_load_image(CRISCPU *cpu, struct cris_load_info
*li)
devboard SDK. */
image_size = load_elf(li->image_filename, NULL,
translate_kernel_address, NULL,
- , NULL, , NULL, 0, EM_CRIS, 0, 0);
+ , NULL, NULL, NULL, 0, EM_CRIS, 0, 0);
li->entry = entry;
if (image_size < 0) {
/* Takes a kimage from the axis devboard SDK. */
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 925e3f7c9d..8ad3c27f2c 100644
---
Re: [PATCH 5/6] macio: don't reference serial_hd() directly within the device
On Sun, 20 Sep 2020, Mark Cave-Ayland wrote:
Instead use qdev_prop_set_chr() to configure the ESCC serial chardevs at the
Mac Old World and New World machine level.
Also remove the now obsolete comment referring to the use of serial_hd() and
change user_createable to true accordingly.
Signed-off-by: Mark Cave-Ayland
---
hw/misc/macio/macio.c | 5 +
hw/ppc/mac_newworld.c | 6 ++
hw/ppc/mac_oldworld.c | 6 ++
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 679722628e..ce641d41fd 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -109,8 +109,6 @@ static void macio_common_realize(PCIDevice *d, Error **errp)
qdev_prop_set_uint32(DEVICE(>escc), "disabled", 0);
qdev_prop_set_uint32(DEVICE(>escc), "frequency", ESCC_CLOCK);
qdev_prop_set_uint32(DEVICE(>escc), "it_shift", 4);
-qdev_prop_set_chr(DEVICE(>escc), "chrA", serial_hd(0));
-qdev_prop_set_chr(DEVICE(>escc), "chrB", serial_hd(1));
qdev_prop_set_uint32(DEVICE(>escc), "chnBtype", escc_serial);
qdev_prop_set_uint32(DEVICE(>escc), "chnAtype", escc_serial);
if (!qdev_realize(DEVICE(>escc), BUS(>macio_bus), errp)) {
@@ -458,8 +456,7 @@ static void macio_class_init(ObjectClass *klass, void *data)
k->class_id = PCI_CLASS_OTHERS << 8;
device_class_set_props(dc, macio_properties);
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-/* Reason: Uses serial_hds in macio_instance_init */
-dc->user_creatable = false;
+dc->user_creatable = true;
According to a comment in
https://git.qemu.org/?p=qemu.git;a=blob;f=hw/core/qdev.c;h=96772a15bd5b76d3ebe27d45ed1f2c1beb7f5386;hb=HEAD#l1135
user_creatable = true is the default and most devices don't set it
explicitely so I think you can just remove the line setting it here.
Regards,
BALATON Zoltan
}
static const TypeInfo macio_bus_info = {
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index e42bd7a626..e59b30e0a7 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -123,6 +123,7 @@ static void ppc_core99_init(MachineState *machine)
UNINHostState *uninorth_pci;
PCIBus *pci_bus;
PCIDevice *macio;
+ESCCState *escc;
bool has_pmu, has_adb;
MACIOIDEState *macio_ide;
BusState *adb_bus;
@@ -382,6 +383,11 @@ static void ppc_core99_init(MachineState *machine)
qdev_prop_set_bit(dev, "has-adb", has_adb);
object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
_abort);
+
+escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
+qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
+qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
+
pci_realize_and_unref(macio, pci_bus, _fatal);
/* We only emulate 2 out of 3 IDE controllers for now */
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 7aba040f1b..25ade63ba3 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -96,6 +96,7 @@ static void ppc_heathrow_init(MachineState *machine)
PCIBus *pci_bus;
PCIDevice *macio;
MACIOIDEState *macio_ide;
+ESCCState *escc;
SysBusDevice *s;
DeviceState *dev, *pic_dev;
BusState *adb_bus;
@@ -283,6 +284,11 @@ static void ppc_heathrow_init(MachineState *machine)
qdev_prop_set_uint64(dev, "frequency", tbfreq);
object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
_abort);
+
+escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
+qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
+qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
+
pci_realize_and_unref(macio, pci_bus, _fatal);
macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio),
Re: [PATCH] ati-vga: Fix checks in ati_2d_blt() to avoid crash
On Sat, 22 Aug 2020, Philippe Mathieu-Daudé wrote: On 4/6/20 10:34 PM, BALATON Zoltan wrote: In some corner cases (that never happen during normal operation but a malicious guest could program wrong values) pixman functions were called with parameters that result in a crash. Fix this and add more checks to disallow such cases. (Fair) question on IRC. Is this patch fixing CVE-2020-24352? Public on August 14, 2020 Description An out-of-bounds memory access flaw was found in the ATI VGA device implementation of the QEMU emulator. This flaw occurs in the ati_2d_blt() routine while handling MMIO write operations through the ati_mm_write() callback. A malicious guest could use this flaw to crash the QEMU process on the host, resulting in a denial of service. Probably this patch does not fix all possible malicious register writes a guest could do. This was fixing problems reported earlier but then I got some more reports around 5.1.0 freeze about some more overruns which I could not yet look at and nobody else was fixing it either so it's possible some bugs are still left in the checks. However this is hardly security critical as ati-vga is experimental and not fully implemented yet so anyone using it will likely get other problems (such as drivers not loading) before a guest could exploit this. I think QEMU only considers bugs in parts that are used for virtualisation via KVM as security problems so maybe this does not even need a CVE and could be normally reported/discussed on the mailing list. Basically what needs to be done is go through the checks again to verify that we don't pass params to pixman or set_dirty that result in access outside the video ram area. Probably there's still an off by one error or some other mistake. I'll eventually may try to fix it but if anyone is sending a patch earlier that's welcome. I don't have much time for QEMU now. Regards, BALATON Zoltan
