Re: [PATCH 11/13] vt82c686: Support machine-default audiodev with fallback

[Paolo Bonzini]

Il sab 23 set 2023, 14:23 BALATON Zoltan  ha scritto:

> On Sat, 23 Sep 2023, Paolo Bonzini wrote:
> > Signed-off-by: Paolo Bonzini 
> > ---
> > hw/isa/vt82c686.c   |  2 ++
> > hw/mips/fuloong2e.c | 13 ++---
> > hw/ppc/pegasos2.c   | 10 --
> > 3 files changed, 20 insertions(+), 5 deletions(-)
>
> This looks better but I still wonder if this machine audiodev propery is
> needed at all. If there's one -audiodev option specified it's already
> picked up by default devices and if there are more one could use -global
> to set it. Why isn't that enough?
>

Mostly because it's less predictable. Ideally all the state of the emulator
would be visible and settable via explicit links.

You were absolutely right that we still need to keep some level of magic in
softmmu/vl.c to make QEMU easier to use for the command line. However, a
while ago there was an idea of making an alternative binary that is
entirely configurable via QMP, and past work in that direction resulted in
*lots* of cleanups that actually made softmmu/vl.c understandable. While I
am not sure this QMP binary is ever going to happen, I would like to make
it possible to avoid the magic.

If you still want a machine audiodev propery then could the device handle
> it without needing changes to the machine? Like in via_isa_realize() add
>
> if (current_machine->audiodev) {
>  qdev_prop_set_string(DEVICE(pci_dev), "audiodev", machine->audiodev);
> }
>
> before qdev_realize(DEVICE(&s->ac97) then no need to change the device
> creation in board code.
>

No, current_machine should not be used at all outside board code.

Paolo


> Regards,
> BALATON Zoltan
>
> > diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> > index 57bdfb4e78c..3ec8e43708a 100644
> > --- a/hw/isa/vt82c686.c
> > +++ b/hw/isa/vt82c686.c
> > @@ -578,6 +578,8 @@ static void via_isa_init(Object *obj)
> > object_initialize_child(obj, "uhci2", &s->uhci[1],
> TYPE_VT82C686B_USB_UHCI);
> > object_initialize_child(obj, "ac97", &s->ac97, TYPE_VIA_AC97);
> > object_initialize_child(obj, "mc97", &s->mc97, TYPE_VIA_MC97);
> > +
> > +object_property_add_alias(obj, "audiodev", OBJECT(&s->ac97),
> "audiodev");
> > }
> >
> > static const TypeInfo via_isa_info = {
> > diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> > index c827f615f3b..df2be188257 100644
> > --- a/hw/mips/fuloong2e.c
> > +++ b/hw/mips/fuloong2e.c
> > @@ -41,6 +41,7 @@
> > #include "sysemu/reset.h"
> > #include "sysemu/sysemu.h"
> > #include "qemu/error-report.h"
> > +#include "audio/audio.h"
> >
> > #define ENVP_PADDR  0x2000
> > #define ENVP_VADDR  cpu_mips_phys_to_kseg0(NULL, ENVP_PADDR)
> > @@ -295,9 +296,13 @@ static void mips_fuloong2e_init(MachineState
> *machine)
> > pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
> >
> > /* South bridge -> IP5 */
> > -pci_dev = pci_create_simple_multifunction(pci_bus,
> > -
> PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
> > -  TYPE_VT82C686B_ISA);
> > +pci_dev = pci_new_multifunction(PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
> > +TYPE_VT82C686B_ISA);
> > +if (machine->audiodev) {
> > +qdev_prop_set_string(DEVICE(pci_dev), "audiodev",
> machine->audiodev);
> > +}
> > +pci_realize_and_unref(pci_dev, pci_bus, &error_abort);
> > +
> > object_property_add_alias(OBJECT(machine), "rtc-time",
> >
>  object_resolve_path_component(OBJECT(pci_dev),
> > "rtc"),
> > @@ -337,6 +342,8 @@ static void mips_fuloong2e_machine_init(MachineClass
> *mc)
> > mc->default_ram_size = 256 * MiB;
> > mc->default_ram_id = "fuloong2e.ram";
> > mc->minimum_page_bits = 14;
> > +
> > +machine_add_audiodev_property(mc);
> > }
> >
> > DEFINE_MACHINE("fuloong2e", mips_fuloong2e_machine_init)
> > diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
> > index bd397cf2b5c..61c302895c9 100644
> > --- a/hw/ppc/pegasos2.c
> > +++ b/hw/ppc/pegasos2.c
> > @@ -37,6 +37,7 @@
> > #include "qemu/datadir.h"
> > #include "sysemu/device_tree.h"
> > #include "hw/ppc/vof.h"
> > +#include "audio/audio.h"
> >
> > #include 
> >
> > @@ -180,8 +181,11 @@ static void pegasos2_init(MachineState *machine)
> > pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);
> >
> > /* VIA VT8231 South Bridge (multifunction PCI device) */
> > -via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12,
> 0),
> > - TYPE_VT8231_ISA));
> > +via = OBJECT(pci_new_multifunction(PCI_DEVFN(12, 0),
> TYPE_VT8231_ISA));
> > +if (machine->audiodev) {
> > +qdev_prop_set_string(DEVICE(via), "audiodev",
> machine->audiodev);
> > +}
> > +pci_realize_and_unref(PCI_DEVICE(via), pci_bus, &error_abort);
> > for (i = 0; i < PCI_NUM_PINS; i++) {
> > pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
> > }
> > @@ -564,6 +568,

Re: [PATCH 02/13] audio: Require AudioState in AUD_add_capture

[Paolo Bonzini]

Il sab 23 set 2023, 13:49 BALATON Zoltan  ha scritto:

> On Sat, 23 Sep 2023, Paolo Bonzini wrote:
> > From: Martin Kletzander 
> >
> > Since all callers require a valid audiodev this function can now safely
> > abort in case of missing AudioState.
> >
> > Signed-off-by: Martin Kletzander 
> > Message-ID: <
> c6e87e678e914df0f59da2145c2753cdb4a16f63.1650874791.git.mklet...@redhat.com
> >
> > Signed-off-by: Paolo Bonzini 
> > ---
> > audio/audio.c | 7 +++
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/audio/audio.c b/audio/audio.c
> > index 90c7c49d116..42bfa330146 100644
> > --- a/audio/audio.c
> > +++ b/audio/audio.c
> > @@ -1876,10 +1876,9 @@ CaptureVoiceOut *AUD_add_capture(
> > struct capture_callback *cb;
> >
> > if (!s) {
> > -if (!legacy_config) {
> > -dolog("Capturing without setting an audiodev is
> deprecated\n");
> > -}
> > -s = audio_init(NULL, NULL);
> > +error_setg(&error_abort,
> > +   "Capturing without setting an audiodev is not
> supported");
> > +abort();
>
> This looks suspicious to me but I don't know if you can do this. Probably
> Markus can advise. I would use error_report and abort() or error_setg if
> you have an errp then return but this func doesn't seem to have errp.
>

Sure, I will change that.

Paolo


> Regards,
> BALATON Zoltan
>
> > }
> >
> > if (!audio_get_pdo_out(s->dev)->mixing_engine) {
> >
>
>

[PATCH] mips: fix abort on integer overflow

[Mikulas Patocka]

Qemu mips userspace emulation crashes with "qemu: unhandled CPU exception 
0x15 - aborting" when one of the integer arithmetic instructions detects 
an overflow.

This patch fixes it so that it delivers SIGFPE with FPE_INTOVF instead.

Signed-off-by: Mikulas Patocka 
Cc: qemu-sta...@nongnu.org

---
 linux-user/mips/cpu_loop.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Index: qemu/linux-user/mips/cpu_loop.c
===
--- qemu.orig/linux-user/mips/cpu_loop.c
+++ qemu/linux-user/mips/cpu_loop.c
@@ -180,7 +180,9 @@ done_syscall:
 }
 force_sig_fault(TARGET_SIGFPE, si_code, env->active_tc.PC);
 break;
-
+   case EXCP_OVERFLOW:
+do_tr_or_bp(env, BRK_OVERFLOW, false);
+break;
 /* The code below was inspired by the MIPS Linux kernel trap
  * handling code in arch/mips/kernel/traps.c.
  */

Re: [PATCH 11/13] vt82c686: Support machine-default audiodev with fallback

[BALATON Zoltan]

On Sun, 24 Sep 2023, Paolo Bonzini wrote:

Il sab 23 set 2023, 14:23 BALATON Zoltan  ha scritto:


On Sat, 23 Sep 2023, Paolo Bonzini wrote:

Signed-off-by: Paolo Bonzini 
---
hw/isa/vt82c686.c   |  2 ++
hw/mips/fuloong2e.c | 13 ++---
hw/ppc/pegasos2.c   | 10 --
3 files changed, 20 insertions(+), 5 deletions(-)


This looks better but I still wonder if this machine audiodev propery is
needed at all. If there's one -audiodev option specified it's already
picked up by default devices and if there are more one could use -global
to set it. Why isn't that enough?



Mostly because it's less predictable. Ideally all the state of the emulator
would be visible and settable via explicit links.

You were absolutely right that we still need to keep some level of magic in
softmmu/vl.c to make QEMU easier to use for the command line. However, a
while ago there was an idea of making an alternative binary that is
entirely configurable via QMP, and past work in that direction resulted in
*lots* of cleanups that actually made softmmu/vl.c understandable. While I
am not sure this QMP binary is ever going to happen, I would like to make
it possible to avoid the magic.

If you still want a machine audiodev propery then could the device handle

it without needing changes to the machine? Like in via_isa_realize() add

if (current_machine->audiodev) {
 qdev_prop_set_string(DEVICE(pci_dev), "audiodev", machine->audiodev);
}

before qdev_realize(DEVICE(&s->ac97) then no need to change the device
creation in board code.



No, current_machine should not be used at all outside board code.


OK, can you start from pci_bus and walk up the QOM tree then to find the 
machine in vt92686.c so the board code does not have to care about this?


Regards,
BALATON Zoltan


diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 57bdfb4e78c..3ec8e43708a 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -578,6 +578,8 @@ static void via_isa_init(Object *obj)
object_initialize_child(obj, "uhci2", &s->uhci[1],

TYPE_VT82C686B_USB_UHCI);

object_initialize_child(obj, "ac97", &s->ac97, TYPE_VIA_AC97);
object_initialize_child(obj, "mc97", &s->mc97, TYPE_VIA_MC97);
+
+object_property_add_alias(obj, "audiodev", OBJECT(&s->ac97),

"audiodev");

}

static const TypeInfo via_isa_info = {
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index c827f615f3b..df2be188257 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -41,6 +41,7 @@
#include "sysemu/reset.h"
#include "sysemu/sysemu.h"
#include "qemu/error-report.h"
+#include "audio/audio.h"

#define ENVP_PADDR  0x2000
#define ENVP_VADDR  cpu_mips_phys_to_kseg0(NULL, ENVP_PADDR)
@@ -295,9 +296,13 @@ static void mips_fuloong2e_init(MachineState

*machine)

pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));

/* South bridge -> IP5 */
-pci_dev = pci_create_simple_multifunction(pci_bus,
-

PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),

-  TYPE_VT82C686B_ISA);
+pci_dev = pci_new_multifunction(PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
+TYPE_VT82C686B_ISA);
+if (machine->audiodev) {
+qdev_prop_set_string(DEVICE(pci_dev), "audiodev",

machine->audiodev);

+}
+pci_realize_and_unref(pci_dev, pci_bus, &error_abort);
+
object_property_add_alias(OBJECT(machine), "rtc-time",


 object_resolve_path_component(OBJECT(pci_dev),

"rtc"),
@@ -337,6 +342,8 @@ static void mips_fuloong2e_machine_init(MachineClass

*mc)

mc->default_ram_size = 256 * MiB;
mc->default_ram_id = "fuloong2e.ram";
mc->minimum_page_bits = 14;
+
+machine_add_audiodev_property(mc);
}

DEFINE_MACHINE("fuloong2e", mips_fuloong2e_machine_init)
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index bd397cf2b5c..61c302895c9 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -37,6 +37,7 @@
#include "qemu/datadir.h"
#include "sysemu/device_tree.h"
#include "hw/ppc/vof.h"
+#include "audio/audio.h"

#include 

@@ -180,8 +181,11 @@ static void pegasos2_init(MachineState *machine)
pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);

/* VIA VT8231 South Bridge (multifunction PCI device) */
-via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12,

0),

- TYPE_VT8231_ISA));
+via = OBJECT(pci_new_multifunction(PCI_DEVFN(12, 0),

TYPE_VT8231_ISA));

+if (machine->audiodev) {
+qdev_prop_set_string(DEVICE(via), "audiodev",

machine->audiodev);

+}
+pci_realize_and_unref(PCI_DEVICE(via), pci_bus, &error_abort);
for (i = 0; i < PCI_NUM_PINS; i++) {
pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
}
@@ -564,6 +568,8 @@ static void pegasos2_machine_class_init(ObjectClass

*oc, void *data)

vhc->encode_hpt_for_kvm_pr = vhyp_encode_hpt_for_kvm_pr;

vmc->setprop = pegasos2_s

Re: [PATCH 3/4] aspeed/i3c: Rename variable shadowing a local

[Philippe Mathieu-Daudé]

On 22/9/23 17:59, Cédric Le Goater wrote:

to fix warning :

   ../hw/i3c/aspeed_i3c.c: In function ‘aspeed_i3c_realize’:
   ../hw/i3c/aspeed_i3c.c:1959:17: warning: declaration of ‘dev’ shadows a 
parameter [-Wshadow=local]
1959 | Object *dev = OBJECT(&s->devices[i]);
 | ^~~
   ../hw/i3c/aspeed_i3c.c:1942:45: note: shadowed declaration is here
1942 | static void aspeed_i3c_realize(DeviceState *dev, Error **errp)
 |~^~~

Signed-off-by: Cédric Le Goater 
---
  hw/misc/aspeed_i3c.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/misc/aspeed_i3c.c b/hw/misc/aspeed_i3c.c
index f54f5da522b3..d1ff61767167 100644
--- a/hw/misc/aspeed_i3c.c
+++ b/hw/misc/aspeed_i3c.c
@@ -296,13 +296,13 @@ static void aspeed_i3c_realize(DeviceState *dev, Error 
**errp)


Alternatively:

-- >8 --

-static void aspeed_i3c_realize(DeviceState *dev, Error **errp)
+static void aspeed_i3c_realize(DeviceState *ctrl, Error **errp)
 {
 int i;
-AspeedI3CState *s = ASPEED_I3C(dev);
-SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+AspeedI3CState *s = ASPEED_I3C(ctrl);
+SysBusDevice *sbd = SYS_BUS_DEVICE(ctrl);

---


  memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);
  
  for (i = 0; i < ASPEED_I3C_NR_DEVICES; ++i) {

-Object *dev = OBJECT(&s->devices[i]);
+Object *i3c_dev = OBJECT(&s->devices[i]);
  
-if (!object_property_set_uint(dev, "device-id", i, errp)) {

+if (!object_property_set_uint(i3c_dev, "device-id", i, errp)) {
  return;
  }
  
-if (!sysbus_realize(SYS_BUS_DEVICE(dev), errp)) {

+if (!sysbus_realize(SYS_BUS_DEVICE(i3c_dev), errp)) {
  return;
  }
  


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v3 2/6] hw/isa/piix3: Reuse piix3_realize() in piix3_xen_realize()

[Bernhard Beschow]

Am 20. September 2023 14:44:23 UTC schrieb Chuck Zmudzinski :
>On 9/19/2023 4:02 PM, Bernhard Beschow wrote:
>> 
>> 
>> Am 3. April 2023 12:27:14 UTC schrieb Jason Andryuk :
>>>On Mon, Apr 3, 2023 at 5:33 AM Anthony PERARD  
>>>wrote:

 On Sat, Apr 01, 2023 at 10:36:45PM +, Bernhard Beschow wrote:
 >
 >
 > Am 30. März 2023 13:00:25 UTC schrieb Anthony PERARD 
 > :
 > >On Sun, Mar 12, 2023 at 01:02:17PM +0100, Bernhard Beschow wrote:
 > >> This is a preparational patch for the next one to make the following
 > >> more obvious:
 > >>
 > >> First, pci_bus_irqs() is now called twice in case of Xen where the
 > >> second call overrides the pci_set_irq_fn with the Xen variant.
 > >
 > >pci_bus_irqs() does allocates pci_bus->irq_count, so the second call in
 > >piix3_xen_realize() will leak `pci_bus->irq_count`. Could you look if
 > >pci_bus_irqs_cleanup() can be called before the second pci_bus_irqs()
 > >call, or maybe some other way to avoid the leak?
 >
 > Thanks for catching this! I'll post a v4.
 >
 > I think the most fool-proof way to fix this is to free irq_count just 
 > before the assignment. pci_bus_irqs_cleanup() would then have to NULL 
 > the attribute such that pci_bus_irqs() can be called afterwards.
 >
 > BTW: I tried running qemu-system-x86_64 with PIIX4 rather than PIIX3 as 
 > Xen guest with my pc-piix4 branch without success. This branch 
 > essentially just provides slightly different PCI IDs for PIIX. Does xl 
 > or something else in Xen check these? If not then this means I'm still 
 > missing something. Under KVM this branch works just fine. Any idea?

 Maybe the ACPI tables provided by libxl needs to be updated.
 Or maybe something in the firmware (SeaBIOS or OVMF/OvmfXen) check the
 id (I know that the PCI id of the root bus is checked, but I don't know
 if that's the one that's been changed).
>>>
>>>Xen also has hvmloader, which runs before SeaBIOS/OVMF.  Looking at
>>>tools/firmware/hvmloader/pci.c, it has
>>>ASSERT((devfn != PCI_ISA_DEVFN) ||
>>>   ((vendor_id == 0x8086) && (device_id == 0x7000)));
>>>
>>>From QEMU, it looks like 0x7000 is PCI_DEVICE_ID_INTEL_82371SB_0, but
>>>PIIX4 uses 0x7110 (PCI_DEVICE_ID_INTEL_82371AB_0).  Maybe try removing
>>>that check?
>> 
>> I was finally able to build Xen successfully (without my distribution 
>> providing too recent dependencies that prevent compilation). With 0x7110 
>> added in the line above I could indeed run a Xen guest with PIIX4. Yay!
>> 
>> Now I just need to respin my PIIX consolidation series...
>
>Welcome to the world of running guests on Xen! I am the one who tested your 
>earlier patches with Xen guests,

Thanks, I remember for sure!

> and I just wanted to say thanks for keeping me in the loop. Please Cc me when 
> you post your respin of the PIIX consolidation series since I would like to 
> also test it in my Xen environment. I understand I will also need to patch 
> hvmloader.c on the Xen side to set the correct device id.

I'd add your e-mail to the recipients list in my Git then.

For those who want a sneak preview of PIIX4 in the PC machine may compile 
https://github.com/shentok/qemu/tree/piix-consolidate and run 
`qemu-system-x86_64 -M pc,south-bridge=piix4-isa`. It should work with all 
available virtualization technologies.

Best regards,
Bernhard

>
>Kind regards,
>
>Chuck
>
>> 
>> Best regards,
>> Bernhard
>> 
>>>
>>>Regards,
>>>Jason
>

Re: [PATCH 11/13] vt82c686: Support machine-default audiodev with fallback

[Paolo Bonzini]

On Sun, Sep 24, 2023 at 2:14 PM BALATON Zoltan  wrote:
> > If you still want a machine audiodev propery then could the device handle
> >> it without needing changes to the machine? Like in via_isa_realize() add
> >>
> >> if (current_machine->audiodev) {
> >>  qdev_prop_set_string(DEVICE(pci_dev), "audiodev", machine->audiodev);
> >> }
> >>
> >> before qdev_realize(DEVICE(&s->ac97) then no need to change the device
> >> creation in board code.
> >>
> >
> > No, current_machine should not be used at all outside board code.
>
> OK, can you start from pci_bus and walk up the QOM tree then to find the
> machine in vt92686.c so the board code does not have to care about this?

The machine itself should not be used outside board code, neither via
current_machine nor by any other means.  There are so few places where
it happens (most of them in fw_cfg) that I'm not really willing to
compromise on this. The board sets properties on the devices, it's not
the devices that fetch settings from outside.

Paolo

[PATCH v4] target/riscv: update checks on writing pmpcfg for Smepmp to version 1.0

[Alvin Chang]

Current checks on writing pmpcfg for Smepmp follows Smepmp version
0.9.1. However, Smepmp specification has already been ratified, and
there are some differences between version 0.9.1 and 1.0. In this
commit we update the checks of writing pmpcfg to follow Smepmp version
1.0.

When mseccfg.MML is set, the constraints to modify PMP rules are:
1. Locked rules cannot be removed or modified until a PMP reset, unless
   mseccfg.RLB is set.
2. From Smepmp specification version 1.0, chapter 2 section 4b:
   Adding a rule with executable privileges that either is M-mode-only
   or a locked Shared-Region is not possible and such pmpcfg writes are
   ignored, leaving pmpcfg unchanged.

The commit transfers the value of pmpcfg into the index of the Smepmp
truth table, and checks the rules by aforementioned specification
changes.

Signed-off-by: Alvin Chang 
---
Changes from v3: Modify "epmp_operation" to "smepmp_operation".

Changes from v2: Adopt switch case ranges and numerical order.

Changes from v1: Convert ePMP over to Smepmp.

 target/riscv/pmp.c | 40 
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index a08cd95658..2ebf18c941 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -99,16 +99,40 @@ static void pmp_write_cfg(CPURISCVState *env, uint32_t 
pmp_index, uint8_t val)
 locked = false;
 }
 
-/* mseccfg.MML is set */
-if (MSECCFG_MML_ISSET(env)) {
-/* not adding execute bit */
-if ((val & PMP_LOCK) != 0 && (val & PMP_EXEC) != PMP_EXEC) {
+/*
+ * mseccfg.MML is set. Locked rules cannot be removed or modified
+ * until a PMP reset. Besides, from Smepmp specification version 
1.0
+ * , chapter 2 section 4b says:
+ * Adding a rule with executable privileges that either is
+ * M-mode-only or a locked Shared-Region is not possible and such
+ * pmpcfg writes are ignored, leaving pmpcfg unchanged.
+ */
+if (MSECCFG_MML_ISSET(env) && !pmp_is_locked(env, pmp_index)) {
+/*
+ * Convert the PMP permissions to match the truth table in the
+ * Smepmp spec.
+ */
+const uint8_t smepmp_operation =
+((val & PMP_LOCK) >> 4) | ((val & PMP_READ) << 2) |
+(val & PMP_WRITE) | ((val & PMP_EXEC) >> 2);
+
+switch (smepmp_operation) {
+case 0 ... 8:
 locked = false;
-}
-/* shared region and not adding X bit */
-if ((val & PMP_LOCK) != PMP_LOCK &&
-(val & 0x7) != (PMP_WRITE | PMP_EXEC)) {
+break;
+case 9 ... 11:
+break;
+case 12:
+locked = false;
+break;
+case 13:
+break;
+case 14:
+case 15:
 locked = false;
+break;
+default:
+g_assert_not_reached();
 }
 }
 } else {
-- 
2.34.1

Re: [PATCH 3/4] hw/cpu: Introduce CPUClass::cpu_resolving_type field

[Gavin Shan]

Hi Philippe,

On 9/12/23 08:40, Gavin Shan wrote:

On 9/11/23 19:43, Philippe Mathieu-Daudé wrote:

On 11/9/23 01:28, Gavin Shan wrote:

On 9/8/23 21:22, Philippe Mathieu-Daudé wrote:

Add a field to return the QOM type name of a CPU class.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/core/cpu.h   | 2 ++
  hw/core/cpu-common.c    | 2 +-
  target/alpha/cpu.c  | 1 +
  target/arm/cpu.c    | 1 +
  target/avr/cpu.c    | 1 +
  target/cris/cpu.c   | 1 +
  target/hexagon/cpu.c    | 1 +
  target/hppa/cpu.c   | 1 +
  target/i386/cpu.c   | 1 +
  target/loongarch/cpu.c  | 1 +
  target/m68k/cpu.c   | 1 +
  target/microblaze/cpu.c | 1 +
  target/mips/cpu.c   | 1 +
  target/nios2/cpu.c  | 1 +
  target/openrisc/cpu.c   | 1 +
  target/ppc/cpu_init.c   | 1 +
  target/riscv/cpu.c  | 1 +
  target/rx/cpu.c | 1 +
  target/s390x/cpu.c  | 1 +
  target/sh4/cpu.c    | 1 +
  target/sparc/cpu.c  | 1 +
  target/tricore/cpu.c    | 1 +
  target/xtensa/cpu.c | 1 +
  23 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 129d179937..e469efd409 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -100,6 +100,7 @@ struct SysemuCPUOps;
  /**
   * CPUClass:
+ * @cpu_resolving_type: CPU QOM type name
   * @class_by_name: Callback to map -cpu command line model name to an
   * instantiatable CPU type.
   * @parse_features: Callback to parse command line arguments.
@@ -148,6 +149,7 @@ struct CPUClass {
  DeviceClass parent_class;
  /*< public >*/
+    const char *cpu_resolving_type;
  ObjectClass *(*class_by_name)(const char *cpu_model);
  void (*parse_features)(const char *typename, char *str, Error **errp);


The question is why not use CPU_RESOLVING_TYPE directly? It seems 
CPU_RESOLVING_TYPE
is exactly what you want here.


Unfortunately CPU_RESOLVING_TYPE is target-specific, we want
hw/core/cpu-common.c to be target-agnostic (build once for all
targets). This is particularly important in the context of
heterogeneous QEMU, where a single binary will be able to create
CPUs from different targets.



CPU_RESOLVING_TYPE and CPUClass::cpu_resolving_type is duplicate to
each other. There are two options I can figure out to avoid the
duplication.

(a) move cpu_class_by_name() from hw/core/cpu-common.c to cpu.c, so that
     CPU_RESOLVING_TYPE can be seen. cpu.c::list_cpus() is the example.

(b) remove hw/core/cpu-common.c::cpu_calss_by_name() and squeeze its
     logic to cpu.c::parse_cpu_option() since there are not too much
     users for it. target/arm and target/s390 needs some tweaks so that
     hw/core/cpu-common.c::cpu_calss_by_name() can be removed.

     [gshan@gshan q]$ git grep \ cpu_class_by_name\(
     cpu.c:    oc = cpu_class_by_name(CPU_RESOLVING_TYPE, model_pieces[0]);
     target/arm/arm-qmp-cmds.c:    oc = cpu_class_by_name(TYPE_ARM_CPU, 
model->name);
     target/s390x/cpu_models_sysemu.c:    oc = cpu_class_by_name(TYPE_S390_CPU, 
info->name);

When option (b) is taken, this series to have the checks against @oc
in hw/core/cpu-common.c::cpu_calss_by_name() becomes non-sense. Instead,
we need the same (and complete) checks in CPUClass::class_by_name() for
individual targets. Further more, an inline helper can be provided to do
the check in CPUClass::class_by_name() for individual targets.

    include/hw/core/cpu.h

    static inline bool cpu_class_is_valid(ObjectClass *oc, const char *parent)
    {
    if (!object_class_dynamic_cast(oc, parent) ||
    object_class_is_abstract(oc)) {
    return false;
    }

    return true;
    }



Since my series to make CPU type check unified depends on this series, could
you please share your thoughts? If you don't have bandwidth for this, I can
improve the code based on your thoughts, and include your patches to my series
so that they can be reviewed at once. Please just let me know.

Thanks,
Gavin

Re: [PATCH] migration/dirtyrate: use QEMU_CLOCK_HOST to report start-time

[Yong Huang]

On Tue, Sep 5, 2023 at 5:19 PM Andrei Gudkov 
wrote:

> Currently query-dirty-rate uses QEMU_CLOCK_REALTIME as
> the source for start-time field. This translates to
> clock_gettime(CLOCK_MONOTONIC), i.e. number of seconds
> since host boot. This is not very useful. The only
> reasonable use case of start-time I can imagine is to
> check whether previously completed measurements are
> too old or not. But this makes sense only if start-time
> is reported as host wall-clock time.
>
> This patch replaces source of start-time from
> QEMU_CLOCK_REALTIME to QEMU_CLOCK_HOST.
>
> Signed-off-by: Andrei Gudkov 
> ---
>  qapi/migration.json   |  4 ++--
>  migration/dirtyrate.c | 15 ++-
>  2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 8843e74b59..63deb8e387 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1941,12 +1941,12 @@
>  # 1. Measurement is in progress:
>  #
>  # <- {"status": "measuring", "sample-pages": 512,
> -# "mode": "page-sampling", "start-time": 3665220, "calc-time": 10}
> +# "mode": "page-sampling", "start-time": 1693900454, "calc-time": 10}
>  #
>  # 2. Measurement has been completed:
>  #
>  # <- {"status": "measured", "sample-pages": 512, "dirty-rate": 108,
> -# "mode": "page-sampling", "start-time": 3665220, "calc-time": 10}
> +# "mode": "page-sampling", "start-time": 1693900454, "calc-time": 10}
>  ##
>  { 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index bccb3515e3..0510d68765 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -259,11 +259,10 @@ static struct DirtyRateInfo
> *query_dirty_rate_info(void)
>  return info;
>  }
>
> -static void init_dirtyrate_stat(int64_t start_time,
> -struct DirtyRateConfig config)
> +static void init_dirtyrate_stat(struct DirtyRateConfig config)
>  {
>  DirtyStat.dirty_rate = -1;
> -DirtyStat.start_time = start_time;
> +DirtyStat.start_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) / 1000;
>  DirtyStat.calc_time = config.sample_period_seconds;
>  DirtyStat.sample_pages = config.sample_pages_per_gigabytes;
>
> @@ -600,7 +599,7 @@ static void calculate_dirtyrate_dirty_bitmap(struct
> DirtyRateConfig config)
>  record_dirtypages_bitmap(&dirty_pages, true);
>
>  start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> -DirtyStat.start_time = start_time / 1000;
> +DirtyStat.start_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) / 1000;
>
>  msec = config.sample_period_seconds * 1000;
>  msec = dirty_stat_wait(msec, start_time);
> @@ -628,7 +627,7 @@ static void calculate_dirtyrate_dirty_ring(struct
> DirtyRateConfig config)
>  /* start log sync */
>  global_dirty_log_change(GLOBAL_DIRTY_DIRTY_RATE, true);
>
> -DirtyStat.start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) / 1000;
> +DirtyStat.start_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) / 1000;
>
>  /* calculate vcpu dirtyrate */
>  duration = vcpu_calculate_dirtyrate(config.sample_period_seconds *
> 1000,
> @@ -657,6 +656,7 @@ static void calculate_dirtyrate_sample_vm(struct
> DirtyRateConfig config)
>
>  rcu_read_lock();
>  initial_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +DirtyStat.start_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) / 1000;
>  if (!record_ramblock_hash_info(&block_dinfo, config, &block_count)) {
>  goto out;
>  }
> @@ -664,7 +664,6 @@ static void calculate_dirtyrate_sample_vm(struct
> DirtyRateConfig config)
>
>  msec = config.sample_period_seconds * 1000;
>  msec = dirty_stat_wait(msec, initial_time);
> -DirtyStat.start_time = initial_time / 1000;
>  DirtyStat.calc_time = msec / 1000;
>
>  rcu_read_lock();
> @@ -727,7 +726,6 @@ void qmp_calc_dirty_rate(int64_t calc_time,
>  static struct DirtyRateConfig config;
>  QemuThread thread;
>  int ret;
> -int64_t start_time;
>
>  /*
>   * If the dirty rate is already being measured, don't attempt to
> start.
> @@ -799,8 +797,7 @@ void qmp_calc_dirty_rate(int64_t calc_time,
>   **/
>  dirtyrate_mode = mode;
>
> -start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) / 1000;
> -init_dirtyrate_stat(start_time, config);
> +init_dirtyrate_stat(config);
>
>  qemu_thread_create(&thread, "get_dirtyrate", get_dirtyrate_thread,
> (void *)&config, QEMU_THREAD_DETACHED);
> --
> 2.30.2
>
>

Reviewed-by: Hyman Huang 

-- 
Best regards

Re: [PATCH v3 12/19] target/riscv: move KVM only files to kvm subdir

[Alistair Francis]

On Wed, Sep 20, 2023 at 9:23 PM Daniel Henrique Barboza
 wrote:
>
> Move the files to a 'kvm' dir to promote more code separation between
> accelerators and making our lives easier supporting build options such
> as --disable-tcg.
>
> Rename kvm.c to kvm-cpu.c to keep it in line with its TCG counterpart.
>
> Signed-off-by: Daniel Henrique Barboza 
> Reviewed-by: Andrew Jones 
> Reviewed-by: LIU Zhiwei 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/intc/riscv_aplic.c | 2 +-
>  hw/riscv/virt.c   | 2 +-
>  target/riscv/cpu.c| 2 +-
>  target/riscv/{kvm.c => kvm/kvm-cpu.c} | 0
>  target/riscv/{ => kvm}/kvm_riscv.h| 0
>  target/riscv/kvm/meson.build  | 1 +
>  target/riscv/meson.build  | 2 +-
>  7 files changed, 5 insertions(+), 4 deletions(-)
>  rename target/riscv/{kvm.c => kvm/kvm-cpu.c} (100%)
>  rename target/riscv/{ => kvm}/kvm_riscv.h (100%)
>  create mode 100644 target/riscv/kvm/meson.build
>
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index 99aae8ccbe..c677b5cfbb 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -32,7 +32,7 @@
>  #include "target/riscv/cpu.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/kvm.h"
> -#include "kvm_riscv.h"
> +#include "kvm/kvm_riscv.h"
>  #include "migration/vmstate.h"
>
>  #define APLIC_MAX_IDC  (1UL << 14)
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 5edc1d98d2..9de578c756 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -35,7 +35,7 @@
>  #include "hw/riscv/virt.h"
>  #include "hw/riscv/boot.h"
>  #include "hw/riscv/numa.h"
> -#include "kvm_riscv.h"
> +#include "kvm/kvm_riscv.h"
>  #include "hw/intc/riscv_aclint.h"
>  #include "hw/intc/riscv_aplic.h"
>  #include "hw/intc/riscv_imsic.h"
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index c8a19be1af..51567c2f12 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -33,7 +33,7 @@
>  #include "fpu/softfloat-helpers.h"
>  #include "sysemu/kvm.h"
>  #include "sysemu/tcg.h"
> -#include "kvm_riscv.h"
> +#include "kvm/kvm_riscv.h"
>  #include "tcg/tcg.h"
>
>  /* RISC-V CPU definitions */
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm/kvm-cpu.c
> similarity index 100%
> rename from target/riscv/kvm.c
> rename to target/riscv/kvm/kvm-cpu.c
> diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm/kvm_riscv.h
> similarity index 100%
> rename from target/riscv/kvm_riscv.h
> rename to target/riscv/kvm/kvm_riscv.h
> diff --git a/target/riscv/kvm/meson.build b/target/riscv/kvm/meson.build
> new file mode 100644
> index 00..7e92415091
> --- /dev/null
> +++ b/target/riscv/kvm/meson.build
> @@ -0,0 +1 @@
> +riscv_ss.add(when: 'CONFIG_KVM', if_true: files('kvm-cpu.c'))
> diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> index 3323b78b84..c53962215f 100644
> --- a/target/riscv/meson.build
> +++ b/target/riscv/meson.build
> @@ -24,7 +24,6 @@ riscv_ss.add(files(
>'zce_helper.c',
>'vcrypto_helper.c'
>  ))
> -riscv_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'))
>
>  riscv_system_ss = ss.source_set()
>  riscv_system_ss.add(files(
> @@ -39,6 +38,7 @@ riscv_system_ss.add(files(
>  ))
>
>  subdir('tcg')
> +subdir('kvm')
>
>  target_arch += {'riscv': riscv_ss}
>  target_softmmu_arch += {'riscv': riscv_system_ss}
> --
> 2.41.0
>
>

Re: [PATCH v3 13/19] target/riscv/kvm: do not use riscv_cpu_add_misa_properties()

[Alistair Francis]

On Wed, Sep 20, 2023 at 9:24 PM Daniel Henrique Barboza
 wrote:
>
> riscv_cpu_add_misa_properties() is being used to fill the missing KVM
> MISA properties but it is a TCG helper that was adapted to do so. We'll
> move it to tcg-cpu.c in the next patches, meaning that KVM needs to fill
> the remaining MISA properties on its own.
>
> Do not use riscv_cpu_add_misa_properties(). Let's create a new array
> with all available MISA bits we support that can be read by KVM. The
> array is zero terminate to allow us to iterate through it without
> knowing its size.
>
> Then, inside kvm_riscv_add_cpu_user_properties(), we'll create all KVM
> MISA properties as usual and then use this array to add any missing MISA
> properties with the riscv_cpu_add_kvm_unavail_prop() helper.
>
> Note that we're creating misa_bits[], and not using the existing
> 'riscv_single_letter_exts[]', because the latter is tuned for riscv,isa
> related functions and it doesn't have all MISA bits we support. Commit
> 0e2c377023 ("target/riscv: misa to ISA string conversion fix") has the
> full context.
>
> While we're at it, move both satp and the multi-letter extension
> properties to kvm_riscv_add_cpu_user_properties() as well.
>
> Signed-off-by: Daniel Henrique Barboza 
> Reviewed-by: Andrew Jones 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c |  2 ++
>  target/riscv/cpu.h |  3 ++-
>  target/riscv/kvm/kvm-cpu.c | 22 ++
>  3 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 51567c2f12..665c21af6a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -38,6 +38,8 @@
>
>  /* RISC-V CPU definitions */
>  static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
> +const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
> +  RVC, RVS, RVU, RVH, RVJ, RVG, 0};
>
>  struct isa_ext_data {
>  const char *name;
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index cb13464ba6..7235eafc1a 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -43,7 +43,7 @@
>  #define RV(x) ((target_ulong)1 << (x - 'A'))
>
>  /*
> - * Consider updating misa_ext_info_arr[] and misa_ext_cfgs[]
> + * Update misa_bits[], misa_ext_info_arr[] and misa_ext_cfgs[]
>   * when adding new MISA bits here.
>   */
>  #define RVI RV('I')
> @@ -60,6 +60,7 @@
>  #define RVJ RV('J')
>  #define RVG RV('G')
>
> +extern const uint32_t misa_bits[];
>  const char *riscv_get_misa_ext_name(uint32_t bit);
>  const char *riscv_get_misa_ext_description(uint32_t bit);
>
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 606fdab223..c6615cb807 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -396,6 +396,8 @@ static void kvm_riscv_add_cpu_user_properties(Object 
> *cpu_obj)
>  {
>  int i;
>
> +riscv_add_satp_mode_properties(cpu_obj);
> +
>  for (i = 0; i < ARRAY_SIZE(kvm_misa_ext_cfgs); i++) {
>  KVMCPUConfig *misa_cfg = &kvm_misa_ext_cfgs[i];
>  int bit = misa_cfg->offset;
> @@ -411,6 +413,11 @@ static void kvm_riscv_add_cpu_user_properties(Object 
> *cpu_obj)
>  misa_cfg->description);
>  }
>
> +for (i = 0; misa_bits[i] != 0; i++) {
> +const char *ext_name = riscv_get_misa_ext_name(misa_bits[i]);
> +riscv_cpu_add_kvm_unavail_prop(cpu_obj, ext_name);
> +}
> +
>  for (i = 0; i < ARRAY_SIZE(kvm_multi_ext_cfgs); i++) {
>  KVMCPUConfig *multi_cfg = &kvm_multi_ext_cfgs[i];
>
> @@ -427,6 +434,10 @@ static void kvm_riscv_add_cpu_user_properties(Object 
> *cpu_obj)
>  object_property_add(cpu_obj, "cboz_blocksize", "uint16",
>  NULL, kvm_cpu_set_cbomz_blksize,
>  NULL, &kvm_cboz_blocksize);
> +
> +riscv_cpu_add_kvm_unavail_prop_array(cpu_obj, riscv_cpu_extensions);
> +riscv_cpu_add_kvm_unavail_prop_array(cpu_obj, riscv_cpu_vendor_exts);
> +riscv_cpu_add_kvm_unavail_prop_array(cpu_obj, 
> riscv_cpu_experimental_exts);
>  }
>
>  static int kvm_riscv_get_regs_core(CPUState *cs)
> @@ -801,7 +812,7 @@ static void kvm_riscv_init_multiext_cfg(RISCVCPU *cpu, 
> KVMScratchCPU *kvmcpu)
>  }
>  }
>
> -static void riscv_init_user_properties(Object *cpu_obj)
> +static void riscv_init_kvm_registers(Object *cpu_obj)
>  {
>  RISCVCPU *cpu = RISCV_CPU(cpu_obj);
>  KVMScratchCPU kvmcpu;
> @@ -810,7 +821,6 @@ static void riscv_init_user_properties(Object *cpu_obj)
>  return;
>  }
>
> -kvm_riscv_add_cpu_user_properties(cpu_obj);
>  kvm_riscv_init_machine_ids(cpu, &kvmcpu);
>  kvm_riscv_init_misa_ext_mask(cpu, &kvmcpu);
>  kvm_riscv_init_multiext_cfg(cpu, &kvmcpu);
> @@ -1324,13 +1334,9 @@ static void kvm_cpu_instance_init(CPUState *cs)
>  Object *obj = OBJECT(RISCV_CPU(cs));
>  DeviceState *dev = DEVICE(obj);
>
> -riscv_init_user_properties(obj);
> -   

Re: [PATCH v3 14/19] target/riscv/cpu.c: export set_misa()

[Alistair Francis]

On Wed, Sep 20, 2023 at 9:23 PM Daniel Henrique Barboza
 wrote:
>
> We'll move riscv_init_max_cpu_extensions() to tcg-cpu.c in the next
> patch and set_misa() needs to be usable from there.
>
> Rename it to riscv_cpu_set_misa() and make it public.
>
> Signed-off-by: Daniel Henrique Barboza 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Andrew Jones 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c | 34 ++
>  target/riscv/cpu.h |  1 +
>  2 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 665c21af6a..cf191d576e 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -294,7 +294,7 @@ const char *riscv_cpu_get_trap_name(target_ulong cause, 
> bool async)
>  }
>  }
>
> -static void set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext)
> +void riscv_cpu_set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext)
>  {
>  env->misa_mxl_max = env->misa_mxl = mxl;
>  env->misa_ext_mask = env->misa_ext = ext;
> @@ -399,9 +399,9 @@ static void riscv_any_cpu_init(Object *obj)
>  RISCVCPU *cpu = RISCV_CPU(obj);
>  CPURISCVState *env = &cpu->env;
>  #if defined(TARGET_RISCV32)
> -set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> +riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | 
> RVU);
>  #elif defined(TARGET_RISCV64)
> -set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
> +riscv_cpu_set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | 
> RVU);
>  #endif
>
>  #ifndef CONFIG_USER_ONLY
> @@ -428,7 +428,7 @@ static void riscv_max_cpu_init(Object *obj)
>  #ifdef TARGET_RISCV32
>  mlx = MXL_RV32;
>  #endif
> -set_misa(env, mlx, 0);
> +riscv_cpu_set_misa(env, mlx, 0);
>  env->priv_ver = PRIV_VERSION_LATEST;
>  #ifndef CONFIG_USER_ONLY
>  set_satp_mode_max_supported(RISCV_CPU(obj), mlx == MXL_RV32 ?
> @@ -441,7 +441,7 @@ static void rv64_base_cpu_init(Object *obj)
>  {
>  CPURISCVState *env = &RISCV_CPU(obj)->env;
>  /* We set this in the realise function */
> -set_misa(env, MXL_RV64, 0);
> +riscv_cpu_set_misa(env, MXL_RV64, 0);
>  /* Set latest version of privileged specification */
>  env->priv_ver = PRIV_VERSION_LATEST;
>  #ifndef CONFIG_USER_ONLY
> @@ -453,7 +453,8 @@ static void rv64_sifive_u_cpu_init(Object *obj)
>  {
>  RISCVCPU *cpu = RISCV_CPU(obj);
>  CPURISCVState *env = &cpu->env;
> -set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> +riscv_cpu_set_misa(env, MXL_RV64,
> +   RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
>  env->priv_ver = PRIV_VERSION_1_10_0;
>  #ifndef CONFIG_USER_ONLY
>  set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV39);
> @@ -471,7 +472,7 @@ static void rv64_sifive_e_cpu_init(Object *obj)
>  CPURISCVState *env = &RISCV_CPU(obj)->env;
>  RISCVCPU *cpu = RISCV_CPU(obj);
>
> -set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
> +riscv_cpu_set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
>  env->priv_ver = PRIV_VERSION_1_10_0;
>  #ifndef CONFIG_USER_ONLY
>  set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> @@ -488,7 +489,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
>  CPURISCVState *env = &RISCV_CPU(obj)->env;
>  RISCVCPU *cpu = RISCV_CPU(obj);
>
> -set_misa(env, MXL_RV64, RVG | RVC | RVS | RVU);
> +riscv_cpu_set_misa(env, MXL_RV64, RVG | RVC | RVS | RVU);
>  env->priv_ver = PRIV_VERSION_1_11_0;
>
>  cpu->cfg.ext_zfa = true;
> @@ -519,7 +520,7 @@ static void rv64_veyron_v1_cpu_init(Object *obj)
>  CPURISCVState *env = &RISCV_CPU(obj)->env;
>  RISCVCPU *cpu = RISCV_CPU(obj);
>
> -set_misa(env, MXL_RV64, RVG | RVC | RVS | RVU | RVH);
> +riscv_cpu_set_misa(env, MXL_RV64, RVG | RVC | RVS | RVU | RVH);
>  env->priv_ver = PRIV_VERSION_1_12_0;
>
>  /* Enable ISA extensions */
> @@ -564,7 +565,7 @@ static void rv128_base_cpu_init(Object *obj)
>  }
>  CPURISCVState *env = &RISCV_CPU(obj)->env;
>  /* We set this in the realise function */
> -set_misa(env, MXL_RV128, 0);
> +riscv_cpu_set_misa(env, MXL_RV128, 0);
>  /* Set latest version of privileged specification */
>  env->priv_ver = PRIV_VERSION_LATEST;
>  #ifndef CONFIG_USER_ONLY
> @@ -576,7 +577,7 @@ static void rv32_base_cpu_init(Object *obj)
>  {
>  CPURISCVState *env = &RISCV_CPU(obj)->env;
>  /* We set this in the realise function */
> -set_misa(env, MXL_RV32, 0);
> +riscv_cpu_set_misa(env, MXL_RV32, 0);
>  /* Set latest version of privileged specification */
>  env->priv_ver = PRIV_VERSION_LATEST;
>  #ifndef CONFIG_USER_ONLY
> @@ -588,7 +589,8 @@ static void rv32_sifive_u_cpu_init(Object *obj)
>  {
>  RISCVCPU *cpu = RISCV_CPU(obj);
>  CPURISCVState *env = &cpu->env;
> -set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> +ris

Re: [PATCH v3 16/19] target/riscv/cpu.c: make misa_ext_cfgs[] 'const'

[Alistair Francis]

On Wed, Sep 20, 2023 at 10:46 PM Daniel Henrique Barboza
 wrote:
>
> The array isn't marked as 'const' because we're initializing their
> elements in riscv_cpu_add_misa_properties(), 'name' and 'description'
> fields.
>
> In a closer look we can see that we're not using these 2 fields after
> creating the MISA properties. And we can create the properties by using
> riscv_get_misa_ext_name() and riscv_get_misa_ext_description()
> directly.
>
> Remove the 'name' and 'description' fields from RISCVCPUMisaExtConfig
> and make misa_ext_cfgs[] a const array.
>
> Signed-off-by: Daniel Henrique Barboza 
> Reviewed-by: Andrew Jones 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c | 21 -
>  1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 8616c9e2f5..4875feded7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1212,8 +1212,6 @@ static void riscv_cpu_init(Object *obj)
>  }
>
>  typedef struct RISCVCPUMisaExtConfig {
> -const char *name;
> -const char *description;
>  target_ulong misa_bit;
>  bool enabled;
>  } RISCVCPUMisaExtConfig;
> @@ -1317,7 +1315,7 @@ const char *riscv_get_misa_ext_description(uint32_t bit)
>  #define MISA_CFG(_bit, _enabled) \
>  {.misa_bit = _bit, .enabled = _enabled}
>
> -static RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
> +static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
>  MISA_CFG(RVA, true),
>  MISA_CFG(RVC, true),
>  MISA_CFG(RVD, true),
> @@ -1344,25 +1342,22 @@ void riscv_cpu_add_misa_properties(Object *cpu_obj)
>  int i;
>
>  for (i = 0; i < ARRAY_SIZE(misa_ext_cfgs); i++) {
> -RISCVCPUMisaExtConfig *misa_cfg = &misa_ext_cfgs[i];
> +const RISCVCPUMisaExtConfig *misa_cfg = &misa_ext_cfgs[i];
>  int bit = misa_cfg->misa_bit;
> -
> -misa_cfg->name = riscv_get_misa_ext_name(bit);
> -misa_cfg->description = riscv_get_misa_ext_description(bit);
> +const char *name = riscv_get_misa_ext_name(bit);
> +const char *desc = riscv_get_misa_ext_description(bit);
>
>  /* Check if KVM already created the property */
> -if (object_property_find(cpu_obj, misa_cfg->name)) {
> +if (object_property_find(cpu_obj, name)) {
>  continue;
>  }
>
> -object_property_add(cpu_obj, misa_cfg->name, "bool",
> +object_property_add(cpu_obj, name, "bool",
>  cpu_get_misa_ext_cfg,
>  cpu_set_misa_ext_cfg,
>  NULL, (void *)misa_cfg);
> -object_property_set_description(cpu_obj, misa_cfg->name,
> -misa_cfg->description);
> -object_property_set_bool(cpu_obj, misa_cfg->name,
> - misa_cfg->enabled, NULL);
> +object_property_set_description(cpu_obj, name, desc);
> +object_property_set_bool(cpu_obj, name, misa_cfg->enabled, NULL);
>  }
>  }
>
> --
> 2.41.0
>
>

Re: [PATCH v4] target/riscv: update checks on writing pmpcfg for Smepmp to version 1.0

[Alistair Francis]

On Mon, Sep 25, 2023 at 2:11 AM Alvin Chang  wrote:
>
> Current checks on writing pmpcfg for Smepmp follows Smepmp version
> 0.9.1. However, Smepmp specification has already been ratified, and
> there are some differences between version 0.9.1 and 1.0. In this
> commit we update the checks of writing pmpcfg to follow Smepmp version
> 1.0.
>
> When mseccfg.MML is set, the constraints to modify PMP rules are:
> 1. Locked rules cannot be removed or modified until a PMP reset, unless
>mseccfg.RLB is set.
> 2. From Smepmp specification version 1.0, chapter 2 section 4b:
>Adding a rule with executable privileges that either is M-mode-only
>or a locked Shared-Region is not possible and such pmpcfg writes are
>ignored, leaving pmpcfg unchanged.
>
> The commit transfers the value of pmpcfg into the index of the Smepmp
> truth table, and checks the rules by aforementioned specification
> changes.
>
> Signed-off-by: Alvin Chang 
> ---
> Changes from v3: Modify "epmp_operation" to "smepmp_operation".

This has the same issue as all the previous versions.

QEMU is currently not shipping with Smepmp support. So updating some
of the code to support Smepmp is confusing.

As I pointed out for the v3, we currently only support ePMP 0.9.3. So
that is what the code must work with.

In order for this change to go in, we also need to upgrade QEMU to
support Smepmp 1.0.

This patch is an attempt to do that:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg967676.html

You basically need to combine the changes from
https://www.mail-archive.com/qemu-devel@nongnu.org/msg967676.html into
this patch. So there is a single patch that updates to Smepmp.

Alistair

>
> Changes from v2: Adopt switch case ranges and numerical order.
>
> Changes from v1: Convert ePMP over to Smepmp.
>
>  target/riscv/pmp.c | 40 
>  1 file changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index a08cd95658..2ebf18c941 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -99,16 +99,40 @@ static void pmp_write_cfg(CPURISCVState *env, uint32_t 
> pmp_index, uint8_t val)
>  locked = false;
>  }
>
> -/* mseccfg.MML is set */
> -if (MSECCFG_MML_ISSET(env)) {
> -/* not adding execute bit */
> -if ((val & PMP_LOCK) != 0 && (val & PMP_EXEC) != PMP_EXEC) {
> +/*
> + * mseccfg.MML is set. Locked rules cannot be removed or modified
> + * until a PMP reset. Besides, from Smepmp specification version 
> 1.0
> + * , chapter 2 section 4b says:
> + * Adding a rule with executable privileges that either is
> + * M-mode-only or a locked Shared-Region is not possible and such
> + * pmpcfg writes are ignored, leaving pmpcfg unchanged.
> + */
> +if (MSECCFG_MML_ISSET(env) && !pmp_is_locked(env, pmp_index)) {
> +/*
> + * Convert the PMP permissions to match the truth table in 
> the
> + * Smepmp spec.
> + */
> +const uint8_t smepmp_operation =
> +((val & PMP_LOCK) >> 4) | ((val & PMP_READ) << 2) |
> +(val & PMP_WRITE) | ((val & PMP_EXEC) >> 2);
> +
> +switch (smepmp_operation) {
> +case 0 ... 8:
>  locked = false;
> -}
> -/* shared region and not adding X bit */
> -if ((val & PMP_LOCK) != PMP_LOCK &&
> -(val & 0x7) != (PMP_WRITE | PMP_EXEC)) {
> +break;
> +case 9 ... 11:
> +break;
> +case 12:
> +locked = false;
> +break;
> +case 13:
> +break;
> +case 14:
> +case 15:
>  locked = false;
> +break;
> +default:
> +g_assert_not_reached();
>  }
>  }
>  } else {
> --
> 2.34.1
>
>

Re: [PATCH v3 15/19] target/riscv/tcg: introduce tcg_cpu_instance_init()

[Alistair Francis]

On Wed, Sep 20, 2023 at 9:23 PM Daniel Henrique Barboza
 wrote:
>
> tcg_cpu_instance_init() will be the 'cpu_instance_init' impl for the TCG
> accelerator. It'll be called from within riscv_cpu_post_init(), via
> accel_cpu_instance_init(), similar to what happens with KVM. In fact, to
> preserve behavior, the implementation will be similar to what
> riscv_cpu_post_init() already does.
>
> In this patch we'll move riscv_cpu_add_user_properties() and
> riscv_init_max_cpu_extensions() and all their dependencies to tcg-cpu.c.
> All multi-extension properties code was moved. The 'multi_ext_user_opts'
> hash table was also moved to tcg-cpu.c since it's a TCG only structure,
> meaning that we won't have to worry about initializing a TCG hash table
> when running a KVM CPU anymore.
>
> riscv_cpu_add_user_properties() will remain in cpu.c for now due to how
> much code it requires to be moved at the same time. We'll do that in the
> next patch.
>
> Signed-off-by: Daniel Henrique Barboza 
> Reviewed-by: Andrew Jones 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c | 150 -
>  target/riscv/cpu.h |   1 -
>  target/riscv/tcg/tcg-cpu.c | 149 
>  3 files changed, 149 insertions(+), 151 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index cf191d576e..8616c9e2f5 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -162,9 +162,6 @@ static const struct isa_ext_data isa_edata_arr[] = {
>  ISA_EXT_DATA_ENTRY(xventanacondops, PRIV_VERSION_1_12_0, 
> ext_XVentanaCondOps),
>  };
>
> -/* Hash that stores user set extensions */
> -static GHashTable *multi_ext_user_opts;
> -
>  bool isa_ext_is_enabled(RISCVCPU *cpu, uint32_t ext_offset)
>  {
>  bool *ext_enabled = (void *)&cpu->cfg + ext_offset;
> @@ -194,12 +191,6 @@ int cpu_cfg_ext_get_min_version(uint32_t ext_offset)
>  g_assert_not_reached();
>  }
>
> -bool cpu_cfg_ext_is_user_set(uint32_t ext_offset)
> -{
> -return g_hash_table_contains(multi_ext_user_opts,
> - GUINT_TO_POINTER(ext_offset));
> -}
> -
>  const char * const riscv_int_regnames[] = {
>  "x0/zero", "x1/ra",  "x2/sp",  "x3/gp",  "x4/tp",  "x5/t0",   "x6/t1",
>  "x7/t2",   "x8/s0",  "x9/s1",  "x10/a0", "x11/a1", "x12/a2",  "x13/a3",
> @@ -280,9 +271,6 @@ static const char * const riscv_intr_names[] = {
>  "reserved"
>  };
>
> -static void riscv_cpu_add_user_properties(Object *obj);
> -static void riscv_init_max_cpu_extensions(Object *obj);
> -
>  const char *riscv_cpu_get_trap_name(target_ulong cause, bool async)
>  {
>  if (async) {
> @@ -1206,32 +1194,9 @@ static bool riscv_cpu_is_dynamic(Object *cpu_obj)
>  return object_dynamic_cast(cpu_obj, TYPE_RISCV_DYNAMIC_CPU) != NULL;
>  }
>
> -static bool riscv_cpu_has_max_extensions(Object *cpu_obj)
> -{
> -return object_dynamic_cast(cpu_obj, TYPE_RISCV_CPU_MAX) != NULL;
> -}
> -
> -static bool riscv_cpu_has_user_properties(Object *cpu_obj)
> -{
> -if (kvm_enabled() &&
> -object_dynamic_cast(cpu_obj, TYPE_RISCV_CPU_HOST) != NULL) {
> -return true;
> -}
> -
> -return riscv_cpu_is_dynamic(cpu_obj);
> -}
> -
>  static void riscv_cpu_post_init(Object *obj)
>  {
>  accel_cpu_instance_init(CPU(obj));
> -
> -if (tcg_enabled() && riscv_cpu_has_user_properties(obj)) {
> -riscv_cpu_add_user_properties(obj);
> -}
> -
> -if (riscv_cpu_has_max_extensions(obj)) {
> -riscv_init_max_cpu_extensions(obj);
> -}
>  }
>
>  static void riscv_cpu_init(Object *obj)
> @@ -1244,8 +1209,6 @@ static void riscv_cpu_init(Object *obj)
>  qdev_init_gpio_in(DEVICE(cpu), riscv_cpu_set_irq,
>IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX);
>  #endif /* CONFIG_USER_ONLY */
> -
> -multi_ext_user_opts = g_hash_table_new(NULL, g_direct_equal);
>  }
>
>  typedef struct RISCVCPUMisaExtConfig {
> @@ -1531,119 +1494,6 @@ Property riscv_cpu_options[] = {
>  DEFINE_PROP_END_OF_LIST(),
>  };
>
> -static void cpu_set_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
> -  void *opaque, Error **errp)
> -{
> -const RISCVCPUMultiExtConfig *multi_ext_cfg = opaque;
> -bool value;
> -
> -if (!visit_type_bool(v, name, &value, errp)) {
> -return;
> -}
> -
> -isa_ext_update_enabled(RISCV_CPU(obj), multi_ext_cfg->offset, value);
> -
> -g_hash_table_insert(multi_ext_user_opts,
> -GUINT_TO_POINTER(multi_ext_cfg->offset),
> -(gpointer)value);
> -}
> -
> -static void cpu_get_multi_ext_cfg(Object *obj, Visitor *v, const char *name,
> -  void *opaque, Error **errp)
> -{
> -const RISCVCPUMultiExtConfig *multi_ext_cfg = opaque;
> -bool value = isa_ext_is_enabled(RISCV_CPU(obj), multi_ext_cfg->offset);
> -
> -visit_type_bool(v, name, &value, errp);
> -}
> -
> -static void cpu

Re: [PATCH v3 17/19] target/riscv/tcg: move riscv_cpu_add_misa_properties() to tcg-cpu.c

[Alistair Francis]

On Wed, Sep 20, 2023 at 10:58 PM Daniel Henrique Barboza
 wrote:
>
> All code related to MISA TCG properties is also moved.
>
> At this point, all TCG properties handling is done in tcg-cpu.c, all KVM
> properties handling is done in kvm-cpu.c.
>
> Signed-off-by: Daniel Henrique Barboza 
> Reviewed-by: Andrew Jones 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c | 90 --
>  target/riscv/cpu.h |  1 -
>  target/riscv/tcg/tcg-cpu.c | 90 ++
>  3 files changed, 90 insertions(+), 91 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 4875feded7..46263e55d5 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1211,47 +1211,6 @@ static void riscv_cpu_init(Object *obj)
>  #endif /* CONFIG_USER_ONLY */
>  }
>
> -typedef struct RISCVCPUMisaExtConfig {
> -target_ulong misa_bit;
> -bool enabled;
> -} RISCVCPUMisaExtConfig;
> -
> -static void cpu_set_misa_ext_cfg(Object *obj, Visitor *v, const char *name,
> - void *opaque, Error **errp)
> -{
> -const RISCVCPUMisaExtConfig *misa_ext_cfg = opaque;
> -target_ulong misa_bit = misa_ext_cfg->misa_bit;
> -RISCVCPU *cpu = RISCV_CPU(obj);
> -CPURISCVState *env = &cpu->env;
> -bool value;
> -
> -if (!visit_type_bool(v, name, &value, errp)) {
> -return;
> -}
> -
> -if (value) {
> -env->misa_ext |= misa_bit;
> -env->misa_ext_mask |= misa_bit;
> -} else {
> -env->misa_ext &= ~misa_bit;
> -env->misa_ext_mask &= ~misa_bit;
> -}
> -}
> -
> -static void cpu_get_misa_ext_cfg(Object *obj, Visitor *v, const char *name,
> - void *opaque, Error **errp)
> -{
> -const RISCVCPUMisaExtConfig *misa_ext_cfg = opaque;
> -target_ulong misa_bit = misa_ext_cfg->misa_bit;
> -RISCVCPU *cpu = RISCV_CPU(obj);
> -CPURISCVState *env = &cpu->env;
> -bool value;
> -
> -value = env->misa_ext & misa_bit;
> -
> -visit_type_bool(v, name, &value, errp);
> -}
> -
>  typedef struct misa_ext_info {
>  const char *name;
>  const char *description;
> @@ -1312,55 +1271,6 @@ const char *riscv_get_misa_ext_description(uint32_t 
> bit)
>  return val;
>  }
>
> -#define MISA_CFG(_bit, _enabled) \
> -{.misa_bit = _bit, .enabled = _enabled}
> -
> -static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
> -MISA_CFG(RVA, true),
> -MISA_CFG(RVC, true),
> -MISA_CFG(RVD, true),
> -MISA_CFG(RVF, true),
> -MISA_CFG(RVI, true),
> -MISA_CFG(RVE, false),
> -MISA_CFG(RVM, true),
> -MISA_CFG(RVS, true),
> -MISA_CFG(RVU, true),
> -MISA_CFG(RVH, true),
> -MISA_CFG(RVJ, false),
> -MISA_CFG(RVV, false),
> -MISA_CFG(RVG, false),
> -};
> -
> -/*
> - * We do not support user choice tracking for MISA
> - * extensions yet because, so far, we do not silently
> - * change MISA bits during realize() (RVG enables MISA
> - * bits but the user is warned about it).
> - */
> -void riscv_cpu_add_misa_properties(Object *cpu_obj)
> -{
> -int i;
> -
> -for (i = 0; i < ARRAY_SIZE(misa_ext_cfgs); i++) {
> -const RISCVCPUMisaExtConfig *misa_cfg = &misa_ext_cfgs[i];
> -int bit = misa_cfg->misa_bit;
> -const char *name = riscv_get_misa_ext_name(bit);
> -const char *desc = riscv_get_misa_ext_description(bit);
> -
> -/* Check if KVM already created the property */
> -if (object_property_find(cpu_obj, name)) {
> -continue;
> -}
> -
> -object_property_add(cpu_obj, name, "bool",
> -cpu_get_misa_ext_cfg,
> -cpu_set_misa_ext_cfg,
> -NULL, (void *)misa_cfg);
> -object_property_set_description(cpu_obj, name, desc);
> -object_property_set_bool(cpu_obj, name, misa_cfg->enabled, NULL);
> -}
> -}
> -
>  #define MULTI_EXT_CFG_BOOL(_name, _prop, _defval) \
>  {.name = _name, .offset = CPU_CFG_OFFSET(_prop), \
>   .enabled = _defval}
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 01cbcbe119..aba8192c74 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -726,7 +726,6 @@ extern const RISCVCPUMultiExtConfig 
> riscv_cpu_vendor_exts[];
>  extern const RISCVCPUMultiExtConfig riscv_cpu_experimental_exts[];
>  extern Property riscv_cpu_options[];
>
> -void riscv_cpu_add_misa_properties(Object *cpu_obj);
>  void riscv_add_satp_mode_properties(Object *obj);
>
>  /* CSR function table */
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 5d71ff2cce..c326ab37a2 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -580,6 +580,96 @@ static bool tcg_cpu_realizefn(CPUState *cs, Error **errp)
>  return true;
>  }
>
> +typedef struct RISCVCPUMisaExtConfig {
> +target_ulong misa_bit;
> +bool enabled;
> +} RISCVCPUMis

Re: [PATCH v3 18/19] target/riscv/cpu.c: export isa_edata_arr[]

[Alistair Francis]

On Wed, Sep 20, 2023 at 9:23 PM Daniel Henrique Barboza
 wrote:
>
> This array will be read by the TCG accel class, allowing it to handle
> priv spec verifications on its own. The array will remain here in cpu.c
> because it's also used by the riscv,isa string function.
>
> To export it we'll finish it with an empty element since ARRAY_SIZE()
> won't work outside of cpu.c. Get rid of its ARRAY_SIZE() usage now to
> alleviate the changes for the next patch.
>
> Signed-off-by: Daniel Henrique Barboza 
> Reviewed-by: Andrew Jones 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c | 47 +-
>  target/riscv/cpu.h |  7 +++
>  2 files changed, 28 insertions(+), 26 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 46263e55d5..e97ba3df93 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -41,15 +41,6 @@ static const char riscv_single_letter_exts[] = 
> "IEMAFDQCPVH";
>  const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
>RVC, RVS, RVU, RVH, RVJ, RVG, 0};
>
> -struct isa_ext_data {
> -const char *name;
> -int min_version;
> -int ext_enable_offset;
> -};
> -
> -#define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \
> -{#_name, _min_ver, CPU_CFG_OFFSET(_prop)}
> -
>  /*
>   * From vector_helper.c
>   * Note that vector data is stored in host-endian 64-bit chunks,
> @@ -61,6 +52,9 @@ struct isa_ext_data {
>  #define BYTE(x)   (x)
>  #endif
>
> +#define ISA_EXT_DATA_ENTRY(_name, _min_ver, _prop) \
> +{#_name, _min_ver, CPU_CFG_OFFSET(_prop)}
> +
>  /*
>   * Here are the ordering rules of extension naming defined by RISC-V
>   * specification :
> @@ -81,7 +75,7 @@ struct isa_ext_data {
>   * Single letter extensions are checked in riscv_cpu_validate_misa_priv()
>   * instead.
>   */
> -static const struct isa_ext_data isa_edata_arr[] = {
> +const RISCVIsaExtData isa_edata_arr[] = {
>  ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_icbom),
>  ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_icboz),
>  ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond),
> @@ -160,6 +154,8 @@ static const struct isa_ext_data isa_edata_arr[] = {
>  ISA_EXT_DATA_ENTRY(xtheadmempair, PRIV_VERSION_1_11_0, 
> ext_xtheadmempair),
>  ISA_EXT_DATA_ENTRY(xtheadsync, PRIV_VERSION_1_11_0, ext_xtheadsync),
>  ISA_EXT_DATA_ENTRY(xventanacondops, PRIV_VERSION_1_12_0, 
> ext_XVentanaCondOps),
> +
> +DEFINE_PROP_END_OF_LIST(),
>  };
>
>  bool isa_ext_is_enabled(RISCVCPU *cpu, uint32_t ext_offset)
> @@ -178,14 +174,14 @@ void isa_ext_update_enabled(RISCVCPU *cpu, uint32_t 
> ext_offset, bool en)
>
>  int cpu_cfg_ext_get_min_version(uint32_t ext_offset)
>  {
> -int i;
> +const RISCVIsaExtData *edata;
>
> -for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
> -if (isa_edata_arr[i].ext_enable_offset != ext_offset) {
> +for (edata = isa_edata_arr; edata && edata->name; edata++) {
> +if (edata->ext_enable_offset != ext_offset) {
>  continue;
>  }
>
> -return isa_edata_arr[i].min_version;
> +return edata->min_version;
>  }
>
>  g_assert_not_reached();
> @@ -932,22 +928,21 @@ static void riscv_cpu_disas_set_info(CPUState *s, 
> disassemble_info *info)
>  void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
>  {
>  CPURISCVState *env = &cpu->env;
> -int i;
> +const RISCVIsaExtData *edata;
>
>  /* Force disable extensions if priv spec version does not match */
> -for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) {
> -if (isa_ext_is_enabled(cpu, isa_edata_arr[i].ext_enable_offset) &&
> -(env->priv_ver < isa_edata_arr[i].min_version)) {
> -isa_ext_update_enabled(cpu, isa_edata_arr[i].ext_enable_offset,
> -   false);
> +for (edata = isa_edata_arr; edata && edata->name; edata++) {
> +if (isa_ext_is_enabled(cpu, edata->ext_enable_offset) &&
> +(env->priv_ver < edata->min_version)) {
> +isa_ext_update_enabled(cpu, edata->ext_enable_offset, false);
>  #ifndef CONFIG_USER_ONLY
>  warn_report("disabling %s extension for hart 0x" TARGET_FMT_lx
>  " because privilege spec version does not match",
> -isa_edata_arr[i].name, env->mhartid);
> +edata->name, env->mhartid);
>  #else
>  warn_report("disabling %s extension because "
>  "privilege spec version does not match",
> -isa_edata_arr[i].name);
> +edata->name);
>  #endif
>  }
>  }
> @@ -1619,13 +1614,13 @@ static void riscv_cpu_class_init(ObjectClass *c, void 
> *data)
>  static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str,
>   int max_str_len)
>  {
> +const RISCVIsaExtData *edata;
>  char *old

Re: [PATCH v3 19/19] target/riscv/cpu: move priv spec functions to tcg-cpu.c

[Alistair Francis]

On Wed, Sep 20, 2023 at 9:24 PM Daniel Henrique Barboza
 wrote:
>
> Priv spec validation is TCG specific. Move it to the TCG accel class.
>
> Signed-off-by: Daniel Henrique Barboza 
> Reviewed-by: Andrew Jones 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c | 38 --
>  target/riscv/cpu.h |  2 --
>  target/riscv/tcg/tcg-cpu.c | 38 ++
>  3 files changed, 38 insertions(+), 40 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index e97ba3df93..eeeb08a35a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -172,21 +172,6 @@ void isa_ext_update_enabled(RISCVCPU *cpu, uint32_t 
> ext_offset, bool en)
>  *ext_enabled = en;
>  }
>
> -int cpu_cfg_ext_get_min_version(uint32_t ext_offset)
> -{
> -const RISCVIsaExtData *edata;
> -
> -for (edata = isa_edata_arr; edata && edata->name; edata++) {
> -if (edata->ext_enable_offset != ext_offset) {
> -continue;
> -}
> -
> -return edata->min_version;
> -}
> -
> -g_assert_not_reached();
> -}
> -
>  const char * const riscv_int_regnames[] = {
>  "x0/zero", "x1/ra",  "x2/sp",  "x3/gp",  "x4/tp",  "x5/t0",   "x6/t1",
>  "x7/t2",   "x8/s0",  "x9/s1",  "x10/a0", "x11/a1", "x12/a2",  "x13/a3",
> @@ -925,29 +910,6 @@ static void riscv_cpu_disas_set_info(CPUState *s, 
> disassemble_info *info)
>  }
>  }
>
> -void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
> -{
> -CPURISCVState *env = &cpu->env;
> -const RISCVIsaExtData *edata;
> -
> -/* Force disable extensions if priv spec version does not match */
> -for (edata = isa_edata_arr; edata && edata->name; edata++) {
> -if (isa_ext_is_enabled(cpu, edata->ext_enable_offset) &&
> -(env->priv_ver < edata->min_version)) {
> -isa_ext_update_enabled(cpu, edata->ext_enable_offset, false);
> -#ifndef CONFIG_USER_ONLY
> -warn_report("disabling %s extension for hart 0x" TARGET_FMT_lx
> -" because privilege spec version does not match",
> -edata->name, env->mhartid);
> -#else
> -warn_report("disabling %s extension because "
> -"privilege spec version does not match",
> -edata->name);
> -#endif
> -}
> -}
> -}
> -
>  #ifndef CONFIG_USER_ONLY
>  static void riscv_cpu_satp_mode_finalize(RISCVCPU *cpu, Error **errp)
>  {
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 3dfcd0732f..219fe2e9b5 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -711,9 +711,7 @@ enum riscv_pmu_event_idx {
>  /* used by tcg/tcg-cpu.c*/
>  void isa_ext_update_enabled(RISCVCPU *cpu, uint32_t ext_offset, bool en);
>  bool isa_ext_is_enabled(RISCVCPU *cpu, uint32_t ext_offset);
> -int cpu_cfg_ext_get_min_version(uint32_t ext_offset);
>  void riscv_cpu_set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext);
> -void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu);
>
>  typedef struct RISCVCPUMultiExtConfig {
>  const char *name;
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index c326ab37a2..8c052d6fcd 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -99,6 +99,21 @@ static const struct TCGCPUOps riscv_tcg_ops = {
>  #endif /* !CONFIG_USER_ONLY */
>  };
>
> +static int cpu_cfg_ext_get_min_version(uint32_t ext_offset)
> +{
> +const RISCVIsaExtData *edata;
> +
> +for (edata = isa_edata_arr; edata && edata->name; edata++) {
> +if (edata->ext_enable_offset != ext_offset) {
> +continue;
> +}
> +
> +return edata->min_version;
> +}
> +
> +g_assert_not_reached();
> +}
> +
>  static void cpu_cfg_ext_auto_update(RISCVCPU *cpu, uint32_t ext_offset,
>  bool value)
>  {
> @@ -226,6 +241,29 @@ static void riscv_cpu_validate_v(CPURISCVState *env, 
> RISCVCPUConfig *cfg,
>  }
>  }
>
> +static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
> +{
> +CPURISCVState *env = &cpu->env;
> +const RISCVIsaExtData *edata;
> +
> +/* Force disable extensions if priv spec version does not match */
> +for (edata = isa_edata_arr; edata && edata->name; edata++) {
> +if (isa_ext_is_enabled(cpu, edata->ext_enable_offset) &&
> +(env->priv_ver < edata->min_version)) {
> +isa_ext_update_enabled(cpu, edata->ext_enable_offset, false);
> +#ifndef CONFIG_USER_ONLY
> +warn_report("disabling %s extension for hart 0x" TARGET_FMT_lx
> +" because privilege spec version does not match",
> +edata->name, env->mhartid);
> +#else
> +warn_report("disabling %s extension because "
> +"privilege spec version does not match",
> +edata->name);
> +#endif
> +}
> +}
> +}
> +

[PATCH v4 08/28] bsd-user: Implement host_to_target_rusage and host_to_target_wrusage.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/bsd-proc.c | 54 +
 1 file changed, 54 insertions(+)

diff --git a/bsd-user/bsd-proc.c b/bsd-user/bsd-proc.c
index 19e39a2f76..aa386ff482 100644
--- a/bsd-user/bsd-proc.c
+++ b/bsd-user/bsd-proc.c
@@ -48,3 +48,57 @@ abi_llong host_to_target_rlim(rlim_t rlim)
 return tswap64(rlim);
 }
 
+void h2g_rusage(const struct rusage *rusage,
+struct target_freebsd_rusage *target_rusage)
+{
+__put_user(rusage->ru_utime.tv_sec, &target_rusage->ru_utime.tv_sec);
+__put_user(rusage->ru_utime.tv_usec, &target_rusage->ru_utime.tv_usec);
+
+__put_user(rusage->ru_stime.tv_sec, &target_rusage->ru_stime.tv_sec);
+__put_user(rusage->ru_stime.tv_usec, &target_rusage->ru_stime.tv_usec);
+
+__put_user(rusage->ru_maxrss, &target_rusage->ru_maxrss);
+__put_user(rusage->ru_idrss, &target_rusage->ru_idrss);
+__put_user(rusage->ru_idrss, &target_rusage->ru_idrss);
+__put_user(rusage->ru_isrss, &target_rusage->ru_isrss);
+__put_user(rusage->ru_minflt, &target_rusage->ru_minflt);
+__put_user(rusage->ru_majflt, &target_rusage->ru_majflt);
+__put_user(rusage->ru_nswap, &target_rusage->ru_nswap);
+__put_user(rusage->ru_inblock, &target_rusage->ru_inblock);
+__put_user(rusage->ru_oublock, &target_rusage->ru_oublock);
+__put_user(rusage->ru_msgsnd, &target_rusage->ru_msgsnd);
+__put_user(rusage->ru_msgrcv, &target_rusage->ru_msgrcv);
+__put_user(rusage->ru_nsignals, &target_rusage->ru_nsignals);
+__put_user(rusage->ru_nvcsw, &target_rusage->ru_nvcsw);
+__put_user(rusage->ru_nivcsw, &target_rusage->ru_nivcsw);
+}
+
+abi_long host_to_target_rusage(abi_ulong target_addr,
+const struct rusage *rusage)
+{
+struct target_freebsd_rusage *target_rusage;
+
+if (!lock_user_struct(VERIFY_WRITE, target_rusage, target_addr, 0)) {
+return -TARGET_EFAULT;
+}
+h2g_rusage(rusage, target_rusage);
+unlock_user_struct(target_rusage, target_addr, 1);
+
+return 0;
+}
+
+abi_long host_to_target_wrusage(abi_ulong target_addr,
+const struct __wrusage *wrusage)
+{
+struct target_freebsd__wrusage *target_wrusage;
+
+if (!lock_user_struct(VERIFY_WRITE, target_wrusage, target_addr, 0)) {
+return -TARGET_EFAULT;
+}
+h2g_rusage(&wrusage->wru_self, &target_wrusage->wru_self);
+h2g_rusage(&wrusage->wru_children, &target_wrusage->wru_children);
+unlock_user_struct(target_wrusage, target_addr, 1);
+
+return 0;
+}
+
-- 
2.42.0

[PATCH v4 16/28] bsd-user: Implement get/set[resuid/resgid/sid] and issetugid.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/bsd-proc.h   | 76 +++
 bsd-user/freebsd/os-syscall.c | 28 +
 2 files changed, 104 insertions(+)

diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
index 6ff07c0ac3..a5f301c72f 100644
--- a/bsd-user/bsd-proc.h
+++ b/bsd-user/bsd-proc.h
@@ -286,4 +286,80 @@ static inline abi_long do_bsd_setregid(abi_long arg1, 
abi_long arg2)
 return get_errno(setregid(arg1, arg2));
 }
 
+/* setresgid(2) */
+static inline abi_long do_bsd_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
+{
+return get_errno(setresgid(rgid, egid, sgid));
+}
+
+/* setresuid(2) */
+static inline abi_long do_bsd_setresuid(uid_t ruid, uid_t euid, uid_t suid)
+{
+return get_errno(setresuid(ruid, euid, suid));
+}
+
+/* getresuid(2) */
+static inline abi_long do_bsd_getresuid(abi_ulong arg1, abi_ulong arg2,
+abi_ulong arg3)
+{
+abi_long ret;
+uid_t ruid, euid, suid;
+
+ret = get_errno(getresuid(&ruid, &euid, &suid));
+if (is_error(ret)) {
+return ret;
+}
+if (put_user_s32(ruid, arg1)) {
+return -TARGET_EFAULT;
+}
+if (put_user_s32(euid, arg2)) {
+return -TARGET_EFAULT;
+}
+if (put_user_s32(suid, arg3)) {
+return -TARGET_EFAULT;
+}
+return ret;
+}
+
+/* getresgid(2) */
+static inline abi_long do_bsd_getresgid(abi_ulong arg1, abi_ulong arg2,
+abi_ulong arg3)
+{
+abi_long ret;
+uid_t ruid, euid, suid;
+
+ret = get_errno(getresgid(&ruid, &euid, &suid));
+if (is_error(ret)) {
+return ret;
+}
+if (put_user_s32(ruid, arg1)) {
+return -TARGET_EFAULT;
+}
+if (put_user_s32(euid, arg2)) {
+return -TARGET_EFAULT;
+}
+if (put_user_s32(suid, arg3)) {
+return -TARGET_EFAULT;
+}
+return ret;
+}
+
+/* getsid(2) */
+static inline abi_long do_bsd_getsid(abi_long arg1)
+{
+return get_errno(getsid(arg1));
+}
+
+/* setsid(2) */
+static inline abi_long do_bsd_setsid(void)
+{
+return get_errno(setsid());
+}
+
+/* issetugid(2) */
+static inline abi_long do_bsd_issetugid(void)
+{
+return get_errno(issetugid());
+}
+
 #endif /* !BSD_PROC_H_ */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 7565e69e76..7b51f4f16e 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -315,6 +315,34 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_bsd_setregid(arg1, arg2);
 break;
 
+case TARGET_FREEBSD_NR_getresuid: /* getresuid(2) */
+ret = do_bsd_getresuid(arg1, arg2, arg3);
+break;
+
+case TARGET_FREEBSD_NR_getresgid: /* getresgid(2) */
+ret = do_bsd_getresgid(arg1, arg2, arg3);
+break;
+
+case TARGET_FREEBSD_NR_setresuid: /* setresuid(2) */
+ret = do_bsd_setresuid(arg1, arg2, arg3);
+break;
+
+case TARGET_FREEBSD_NR_setresgid: /* setresgid(2) */
+ret = do_bsd_setresgid(arg1, arg2, arg3);
+break;
+
+case TARGET_FREEBSD_NR_getsid: /* getsid(2) */
+ret = do_bsd_getsid(arg1);
+break;
+
+case TARGET_FREEBSD_NR_setsid: /* setsid(2) */
+ret = do_bsd_setsid();
+break;
+
+case TARGET_FREEBSD_NR_issetugid: /* issetugid(2) */
+ret = do_bsd_issetugid();
+break;
+
 
 /*
  * File system calls.
-- 
2.42.0

[PATCH v4 04/28] bsd-user: Add freebsd_exec_common and do_freebsd_procctl to qemu.h.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/qemu.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index d9507137cc..41c7bd31d3 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -249,6 +249,12 @@ abi_long get_errno(abi_long ret);
 bool is_error(abi_long ret);
 int host_to_target_errno(int err);
 
+/* os-proc.c */
+abi_long freebsd_exec_common(abi_ulong path_or_fd, abi_ulong guest_argp,
+abi_ulong guest_envp, int do_fexec);
+abi_long do_freebsd_procctl(void *cpu_env, int idtype, abi_ulong arg2,
+abi_ulong arg3, abi_ulong arg4, abi_ulong arg5, abi_ulong arg6);
+
 /* os-sys.c */
 abi_long do_freebsd_sysctl(CPUArchState *env, abi_ulong namep, int32_t namelen,
 abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp, abi_ulong newlen);
-- 
2.42.0

[PATCH v4 26/28] bsd-user: Implement fork(2) and vfork(2) system calls.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/freebsd/os-proc.h| 34 ++
 bsd-user/freebsd/os-syscall.c |  8 
 2 files changed, 42 insertions(+)

diff --git a/bsd-user/freebsd/os-proc.h b/bsd-user/freebsd/os-proc.h
index 42bdd61904..7b2e6a9f79 100644
--- a/bsd-user/freebsd/os-proc.h
+++ b/bsd-user/freebsd/os-proc.h
@@ -185,4 +185,38 @@ static inline abi_long do_freebsd___setugid(abi_long arg1)
 return -TARGET_ENOSYS;
 }
 
+/* fork(2) */
+static inline abi_long do_freebsd_fork(void *cpu_env)
+{
+abi_long ret;
+abi_ulong child_flag;
+
+fork_start();
+ret = fork();
+if (ret == 0) {
+/* child */
+child_flag = 1;
+target_cpu_clone_regs(cpu_env, 0);
+} else {
+/* parent */
+child_flag = 0;
+}
+
+/*
+ * The fork system call sets a child flag in the second return
+ * value: 0 for parent process, 1 for child process.
+ */
+set_second_rval(cpu_env, child_flag);
+
+fork_end(child_flag);
+
+return ret;
+}
+
+/* vfork(2) */
+static inline abi_long do_freebsd_vfork(void *cpu_env)
+{
+return do_freebsd_fork(cpu_env);
+}
+
 #endif /* BSD_USER_FREEBSD_OS_PROC_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 99af0f6b15..cb9425c9ba 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -226,6 +226,14 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 /*
  * process system calls
  */
+case TARGET_FREEBSD_NR_fork: /* fork(2) */
+ret = do_freebsd_fork(cpu_env);
+break;
+
+case TARGET_FREEBSD_NR_vfork: /* vfork(2) */
+ret = do_freebsd_vfork(cpu_env);
+break;
+
 case TARGET_FREEBSD_NR_execve: /* execve(2) */
 ret = do_freebsd_execve(arg1, arg2, arg3);
 break;
-- 
2.42.0

[PATCH v4 02/28] bsd-user: Define procctl(2) related structs

[Karim Taha]

From: Stacey Son 

Implement procctl flags and related structs:
struct target_procctl_reaper_status
struct target_procctl_reaper_pidinfo
struct target_procctl_reaper_pids
struct target_procctl_reaper_kill

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/syscall_defs.h | 42 +
 1 file changed, 42 insertions(+)

diff --git a/bsd-user/syscall_defs.h b/bsd-user/syscall_defs.h
index ddd38c13e0..a3bc738ff8 100644
--- a/bsd-user/syscall_defs.h
+++ b/bsd-user/syscall_defs.h
@@ -390,6 +390,48 @@ struct target_freebsd_flock {
 /* user: vfork(2) semantics, clear signals */
 #define TARGET_RFSPAWN (1U << 31)
 
+/*
+ * from sys/procctl.h
+ */
+#define TARGET_PROC_SPROTECT1
+#define TARGET_PROC_REAP_ACQUIRE2
+#define TARGET_PROC_REAP_RELEASE3
+#define TARGET_PROC_REAP_STATUS 4
+#define TARGET_PROC_REAP_GETPIDS5
+#define TARGET_PROC_REAP_KILL   6
+
+struct target_procctl_reaper_status {
+uint32_t rs_flags;
+uint32_t rs_children;
+uint32_t rs_descendants;
+uint32_t rs_reaper;
+uint32_t rs_pid;
+uint32_t rs_pad0[15];
+};
+
+struct target_procctl_reaper_pidinfo {
+uint32_t pi_pid;
+uint32_t pi_subtree;
+uint32_t pi_flags;
+uint32_t pi_pad0[15];
+};
+
+struct target_procctl_reaper_pids {
+uint32_t rp_count;
+uint32_t rp_pad0[15];
+abi_ulong rp_pids;
+};
+
+struct target_procctl_reaper_kill {
+int32_t  rk_sig;
+uint32_t rk_flags;
+uint32_t rk_subtree;
+uint32_t rk_killed;
+uint32_t rk_fpid;
+uint32_t rk_pad0[15];
+};
+
+
 #define safe_syscall0(type, name) \
 type safe_##name(void) \
 { \
-- 
2.42.0

[PATCH v4 05/28] bsd-user: add extern declarations for bsd-proc.c conversion functions

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/qemu-bsd.h | 38 ++
 1 file changed, 38 insertions(+)
 create mode 100644 bsd-user/qemu-bsd.h

diff --git a/bsd-user/qemu-bsd.h b/bsd-user/qemu-bsd.h
new file mode 100644
index 00..b93a0b7fd5
--- /dev/null
+++ b/bsd-user/qemu-bsd.h
@@ -0,0 +1,38 @@
+/*
+ *  BSD conversion extern declarations
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef QEMU_BSD_H
+#define QEMU_BSD_H
+
+#include 
+#include 
+
+/* bsd-proc.c */
+int target_to_host_resource(int code);
+rlim_t target_to_host_rlim(abi_llong target_rlim);
+abi_llong host_to_target_rlim(rlim_t rlim);
+abi_long host_to_target_rusage(abi_ulong target_addr,
+const struct rusage *rusage);
+abi_long host_to_target_wrusage(abi_ulong target_addr,
+const struct __wrusage *wrusage);
+int host_to_target_waitstatus(int status);
+void h2g_rusage(const struct rusage *rusage,
+struct target_freebsd_rusage *target_rusage);
+
+#endif /* QEMU_BSD_H */
-- 
2.42.0

[PATCH v4 00/28] bsd-user: Implement freebsd process related system calls.

[Karim Taha]

Karim Taha (3):
  bsd-user: define TARGET_RFSPAWN for rfork to use vfork(2) semantics,
and fix RLIM_INFINITY
  bsd-user: Implement get_filename_from_fd.
  bsd-user: Implement execve(2) and fexecve(2) system calls.

Kyle Evans (1):
  bsd-user: Get number of cpus.

Stacey Son (24):
  bsd-user: Define procctl(2) related structs
  bsd-user: Implement host_to_target_siginfo.
  bsd-user: Add freebsd_exec_common and do_freebsd_procctl to qemu.h.
  bsd-user: add extern declarations for bsd-proc.c conversion functions
  bsd-user: Implement target_to_host_resource conversion function
  bsd-user: Implement target_to_host_rlim and host_to_target_rlim
conversion.
  bsd-user: Implement host_to_target_rusage and host_to_target_wrusage.
  bsd-user: Implement host_to_target_waitstatus conversion.
  bsd-user: Implement getgroups(2) and setgroups(2) system calls.
  bsd-user: Implement umask(2), setlogin(2) and getlogin(2)
  bsd-user: Implement getrusage(2).
  bsd-user: Implement getrlimit(2) and setrlimit(2)
  bsd-user: Implement several get/set system calls:
  bsd-user: Implement get/set[resuid/resgid/sid] and issetugid.
  bsd-user: Add stubs for profil(2), ktrace(2), utrace(2) and ptrace(2).
  bsd-user: Implement getpriority(2) and setpriority(2).
  bsd-user: Implement freebsd_exec_common, used in implementing
execve/fexecve.
  bsd-user: Implement procctl(2) along with necessary conversion
functions.
  bsd-user: Implement wait4(2) and wait6(2) system calls.
  bsd-user: Implement setloginclass(2) and getloginclass(2) system
calls.
  bsd-user: Implement pdgetpid(2) and the undocumented setugid.
  bsd-user: Implement fork(2) and vfork(2) system calls.
  bsd-user: Implement rfork(2) system call.
  bsd-user: Implement pdfork(2) system call.

 bsd-user/bsd-proc.c   | 145 ++
 bsd-user/bsd-proc.h   | 379 +++
 bsd-user/freebsd/meson.build  |   1 +
 bsd-user/freebsd/os-proc.c| 479 ++
 bsd-user/freebsd/os-proc.h| 293 +
 bsd-user/freebsd/os-syscall.c | 206 ++-
 bsd-user/main.c   |   2 +-
 bsd-user/meson.build  |   6 +
 bsd-user/qemu-bsd.h   |  38 +++
 bsd-user/qemu.h   |   7 +
 bsd-user/signal-common.h  |   1 +
 bsd-user/signal.c |   6 +
 bsd-user/syscall_defs.h   |  50 +++-
 13 files changed, 1607 insertions(+), 6 deletions(-)
 create mode 100644 bsd-user/bsd-proc.c
 create mode 100644 bsd-user/freebsd/os-proc.c
 create mode 100644 bsd-user/freebsd/os-proc.h
 create mode 100644 bsd-user/qemu-bsd.h

-- 
2.42.0

[PATCH v4 13/28] bsd-user: Implement getrusage(2).

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/bsd-proc.h   | 13 +
 bsd-user/freebsd/os-syscall.c |  4 
 2 files changed, 17 insertions(+)

diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
index cb7c69acb0..133c1b0eaf 100644
--- a/bsd-user/bsd-proc.h
+++ b/bsd-user/bsd-proc.h
@@ -124,4 +124,17 @@ static inline abi_long do_bsd_getlogin(abi_long arg1, 
abi_long arg2)
 return ret;
 }
 
+/* getrusage(2) */
+static inline abi_long do_bsd_getrusage(abi_long who, abi_ulong target_addr)
+{
+abi_long ret;
+struct rusage rusage;
+
+ret = get_errno(getrusage(who, &rusage));
+if (!is_error(ret)) {
+host_to_target_rusage(target_addr, &rusage);
+}
+return ret;
+}
+
 #endif /* !BSD_PROC_H_ */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 44cbf52f08..5d8693ed55 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -243,6 +243,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_bsd_getlogin(arg1, arg2);
 break;
 
+case TARGET_FREEBSD_NR_getrusage: /* getrusage(2) */
+ret = do_bsd_getrusage(arg1, arg2);
+break;
+
 
 /*
  * File system calls.
-- 
2.42.0

[PATCH v4 01/28] bsd-user: define TARGET_RFSPAWN for rfork to use vfork(2) semantics, and fix RLIM_INFINITY

[Karim Taha]

RLIM_INFINITY on FreeBSD, OpenBSD and NetBSD has value of ~(1<<63), caculated
one way or another.

Signed-off-by: Kyle Evans 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/syscall_defs.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/bsd-user/syscall_defs.h b/bsd-user/syscall_defs.h
index 9c90616baa..ddd38c13e0 100644
--- a/bsd-user/syscall_defs.h
+++ b/bsd-user/syscall_defs.h
@@ -130,11 +130,7 @@ struct target_freebsd_timeval {
 /*
  *  sys/resource.h
  */
-#if defined(__FreeBSD__)
 #define TARGET_RLIM_INFINITYRLIM_INFINITY
-#else
-#define TARGET_RLIM_INFINITY((abi_ulong)-1)
-#endif
 
 #define TARGET_RLIMIT_CPU   0
 #define TARGET_RLIMIT_FSIZE 1
@@ -390,6 +386,10 @@ struct target_freebsd_flock {
 int32_t l_sysid;
 } QEMU_PACKED;
 
+/* sys/unistd.h */
+/* user: vfork(2) semantics, clear signals */
+#define TARGET_RFSPAWN (1U << 31)
+
 #define safe_syscall0(type, name) \
 type safe_##name(void) \
 { \
-- 
2.42.0

[PATCH v4 03/28] bsd-user: Implement host_to_target_siginfo.

[Karim Taha]

From: Stacey Son 

Used in wait6 system call

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/signal-common.h | 1 +
 bsd-user/signal.c| 6 ++
 2 files changed, 7 insertions(+)

diff --git a/bsd-user/signal-common.h b/bsd-user/signal-common.h
index c044e81165..77d7c7a78b 100644
--- a/bsd-user/signal-common.h
+++ b/bsd-user/signal-common.h
@@ -35,6 +35,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
 abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, abi_ulong sp);
 long do_sigreturn(CPUArchState *env, abi_ulong addr);
 void force_sig_fault(int sig, int code, abi_ulong addr);
+void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info);
 int host_to_target_signal(int sig);
 void host_to_target_sigset(target_sigset_t *d, const sigset_t *s);
 void process_pending_signals(CPUArchState *env);
diff --git a/bsd-user/signal.c b/bsd-user/signal.c
index b6beab659e..ea82241b70 100644
--- a/bsd-user/signal.c
+++ b/bsd-user/signal.c
@@ -311,6 +311,12 @@ static void tswap_siginfo(target_siginfo_t *tinfo, const 
target_siginfo_t *info)
 }
 }
 
+void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info)
+{
+host_to_target_siginfo_noswap(tinfo, info);
+tswap_siginfo(tinfo, tinfo);
+}
+
 int block_signals(void)
 {
 TaskState *ts = (TaskState *)thread_cpu->opaque;
-- 
2.42.0

[PATCH v4 25/28] bsd-user: Implement pdgetpid(2) and the undocumented setugid.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
---
 bsd-user/freebsd/os-proc.h| 23 +++
 bsd-user/freebsd/os-syscall.c |  8 
 2 files changed, 31 insertions(+)

diff --git a/bsd-user/freebsd/os-proc.h b/bsd-user/freebsd/os-proc.h
index 2eaba141dc..42bdd61904 100644
--- a/bsd-user/freebsd/os-proc.h
+++ b/bsd-user/freebsd/os-proc.h
@@ -34,6 +34,8 @@ pid_t safe_wait4(pid_t wpid, int *status, int options, struct 
rusage *rusage);
 pid_t safe_wait6(idtype_t idtype, id_t id, int *status, int options,
 struct __wrusage *wrusage, siginfo_t *infop);
 
+extern int __setugid(int flag);
+
 /* execve(2) */
 static inline abi_long do_freebsd_execve(abi_ulong path_or_fd, abi_ulong argp,
 abi_ulong envp)
@@ -162,4 +164,25 @@ static inline abi_long do_freebsd_getloginclass(abi_ulong 
arg1, abi_ulong arg2)
 return ret;
 }
 
+/* pdgetpid(2) */
+static inline abi_long do_freebsd_pdgetpid(abi_long fd, abi_ulong target_pidp)
+{
+abi_long ret;
+pid_t pid;
+
+ret = get_errno(pdgetpid(fd, &pid));
+if (!is_error(ret)) {
+if (put_user_u32(pid, target_pidp)) {
+return -TARGET_EFAULT;
+}
+}
+return ret;
+}
+
+/* undocumented __setugid */
+static inline abi_long do_freebsd___setugid(abi_long arg1)
+{
+return -TARGET_ENOSYS;
+}
+
 #endif /* BSD_USER_FREEBSD_OS_PROC_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index d614409e69..99af0f6b15 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -383,6 +383,14 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_freebsd_getloginclass(arg1, arg2);
 break;
 
+case TARGET_FREEBSD_NR_pdgetpid: /* pdgetpid(2) */
+ret = do_freebsd_pdgetpid(arg1, arg2);
+break;
+
+case TARGET_FREEBSD_NR___setugid: /* undocumented */
+ret = do_freebsd___setugid(arg1);
+break;
+
 case TARGET_FREEBSD_NR_utrace: /* utrace(2) */
 ret = do_bsd_utrace(arg1, arg2);
 break;
-- 
2.42.0

[PATCH v4 07/28] bsd-user: Implement target_to_host_rlim and host_to_target_rlim conversion.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/bsd-proc.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/bsd-user/bsd-proc.c b/bsd-user/bsd-proc.c
index 68410a0aa9..19e39a2f76 100644
--- a/bsd-user/bsd-proc.c
+++ b/bsd-user/bsd-proc.c
@@ -38,3 +38,13 @@ int target_to_host_resource(int code)
 return code;
 }
 
+rlim_t target_to_host_rlim(abi_llong target_rlim)
+{
+return tswap64(target_rlim);
+}
+
+abi_llong host_to_target_rlim(rlim_t rlim)
+{
+return tswap64(rlim);
+}
+
-- 
2.42.0

[PATCH v4 23/28] bsd-user: Implement wait4(2) and wait6(2) system calls.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
---
 bsd-user/freebsd/os-proc.h| 84 +++
 bsd-user/freebsd/os-syscall.c | 15 +++
 2 files changed, 99 insertions(+)

diff --git a/bsd-user/freebsd/os-proc.h b/bsd-user/freebsd/os-proc.h
index 75ed39f8dd..04bce755e5 100644
--- a/bsd-user/freebsd/os-proc.h
+++ b/bsd-user/freebsd/os-proc.h
@@ -30,6 +30,10 @@
 
 #include "target_arch_cpu.h"
 
+pid_t safe_wait4(pid_t wpid, int *status, int options, struct rusage *rusage);
+pid_t safe_wait6(idtype_t idtype, id_t id, int *status, int options,
+struct __wrusage *wrusage, siginfo_t *infop);
+
 /* execve(2) */
 static inline abi_long do_freebsd_execve(abi_ulong path_or_fd, abi_ulong argp,
 abi_ulong envp)
@@ -46,4 +50,84 @@ static inline abi_long do_freebsd_fexecve(abi_ulong 
path_or_fd, abi_ulong argp,
 return freebsd_exec_common(path_or_fd, argp, envp, 1);
 }
 
+/* wait4(2) */
+static inline abi_long do_freebsd_wait4(abi_long arg1, abi_ulong target_status,
+abi_long arg3, abi_ulong target_rusage)
+{
+abi_long ret;
+int status;
+struct rusage rusage, *rusage_ptr = NULL;
+
+if (target_rusage) {
+rusage_ptr = &rusage;
+}
+ret = get_errno(safe_wait4(arg1, &status, arg3, rusage_ptr));
+
+if (ret < 0) {
+return ret;
+}
+if (target_status != 0) {
+status = host_to_target_waitstatus(status);
+if (put_user_s32(status, target_status) != 0) {
+return -TARGET_EFAULT;
+}
+}
+if (target_rusage != 0) {
+host_to_target_rusage(target_rusage, &rusage);
+}
+return ret;
+}
+
+/* wait6(2) */
+static inline abi_long do_freebsd_wait6(void *cpu_env, abi_long idtype,
+abi_long id1, abi_long id2,
+abi_ulong target_status, abi_long options, abi_ulong target_wrusage,
+abi_ulong target_infop, abi_ulong pad1)
+{
+abi_long ret;
+int status;
+struct __wrusage wrusage, *wrusage_ptr = NULL;
+siginfo_t info;
+void *p;
+
+if (regpairs_aligned(cpu_env) != 0) {
+/* printf("shifting args\n"); */
+/* 64-bit id is aligned, so shift all the arguments over by one */
+id1 = id2;
+id2 = target_status;
+target_status = options;
+options = target_wrusage;
+target_wrusage = target_infop;
+target_infop = pad1;
+}
+
+if (target_wrusage) {
+wrusage_ptr = &wrusage;
+}
+ret = get_errno(safe_wait6(idtype, target_arg64(id1, id2),
+   &status, options, wrusage_ptr, &info));
+
+if (ret < 0) {
+return ret;
+}
+if (target_status != 0) {
+status = host_to_target_waitstatus(status);
+if (put_user_s32(status, target_status) != 0) {
+return -TARGET_EFAULT;
+}
+}
+if (target_wrusage != 0) {
+host_to_target_wrusage(target_wrusage, &wrusage);
+}
+if (target_infop != 0) {
+p = lock_user(VERIFY_WRITE, target_infop, sizeof(target_siginfo_t), 0);
+if (p == NULL) {
+return -TARGET_EFAULT;
+}
+host_to_target_siginfo(p, &info);
+unlock_user(p, target_infop, sizeof(target_siginfo_t));
+}
+return ret;
+}
+
 #endif /* BSD_USER_FREEBSD_OS_PROC_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 515eaaf31f..55e68e4815 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -40,6 +40,12 @@
 #include "os-stat.h"
 #include "os-proc.h"
 
+/* used in os-proc */
+safe_syscall4(pid_t, wait4, pid_t, wpid, int *, status, int, options,
+struct rusage *, rusage);
+safe_syscall6(pid_t, wait6, idtype_t, idtype, id_t, id, int *, status, int,
+options, struct __wrusage *, wrusage, siginfo_t *, infop);
+
 /* I/O */
 safe_syscall3(int, open, const char *, path, int, flags, mode_t, mode);
 safe_syscall4(int, openat, int, fd, const char *, path, int, flags, mode_t,
@@ -228,6 +234,15 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_freebsd_fexecve(arg1, arg2, arg3);
 break;
 
+case TARGET_FREEBSD_NR_wait4: /* wait4(2) */
+ret = do_freebsd_wait4(arg1, arg2, arg3, arg4);
+break;
+
+case TARGET_FREEBSD_NR_wait6: /* wait6(2) */
+ret = do_freebsd_wait6(cpu_env, arg1, arg2, arg3,
+   arg4, arg5, arg6, arg7, arg8);
+break;
+
 case TARGET_FREEBSD_NR_exit: /* exit(2) */
 ret = do_bsd_exit(cpu_env, arg1);
 break;
-- 
2.42.0

[PATCH v4 15/28] bsd-user: Implement several get/set system calls:

[Karim Taha]

From: Stacey Son 

getpid(2), getppid(2), getpgrp(2)
setreuid(2), setregid(2)
getuid(2), geteuid(2), getgid(2), getegid(2), getpgid(2)
setuid(2), seteuid(2), setgid(2), setegid(2), setpgid(2)

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/bsd-proc.h   | 90 +++
 bsd-user/freebsd/os-syscall.c | 60 +++
 2 files changed, 150 insertions(+)

diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
index 38d1324034..6ff07c0ac3 100644
--- a/bsd-user/bsd-proc.h
+++ b/bsd-user/bsd-proc.h
@@ -196,4 +196,94 @@ static inline abi_long do_bsd_setrlimit(abi_long arg1, 
abi_ulong arg2)
 return ret;
 }
 
+/* getpid(2) */
+static inline abi_long do_bsd_getpid(void)
+{
+return get_errno(getpid());
+}
+
+/* getppid(2) */
+static inline abi_long do_bsd_getppid(void)
+{
+return get_errno(getppid());
+}
+
+/* getuid(2) */
+static inline abi_long do_bsd_getuid(void)
+{
+return get_errno(getuid());
+}
+
+/* geteuid(2) */
+static inline abi_long do_bsd_geteuid(void)
+{
+return get_errno(geteuid());
+}
+
+/* getgid(2) */
+static inline abi_long do_bsd_getgid(void)
+{
+return get_errno(getgid());
+}
+
+/* getegid(2) */
+static inline abi_long do_bsd_getegid(void)
+{
+return get_errno(getegid());
+}
+
+/* setuid(2) */
+static inline abi_long do_bsd_setuid(abi_long arg1)
+{
+return get_errno(setuid(arg1));
+}
+
+/* seteuid(2) */
+static inline abi_long do_bsd_seteuid(abi_long arg1)
+{
+return get_errno(seteuid(arg1));
+}
+
+/* setgid(2) */
+static inline abi_long do_bsd_setgid(abi_long arg1)
+{
+return get_errno(setgid(arg1));
+}
+
+/* setegid(2) */
+static inline abi_long do_bsd_setegid(abi_long arg1)
+{
+return get_errno(setegid(arg1));
+}
+
+/* getpgid(2) */
+static inline abi_long do_bsd_getpgid(pid_t pid)
+{
+return get_errno(getpgid(pid));
+}
+
+/* setpgid(2) */
+static inline abi_long do_bsd_setpgid(int pid, int pgrp)
+{
+return get_errno(setpgid(pid, pgrp));
+}
+
+/* getpgrp(2) */
+static inline abi_long do_bsd_getpgrp(void)
+{
+return get_errno(getpgrp());
+}
+
+/* setreuid(2) */
+static inline abi_long do_bsd_setreuid(abi_long arg1, abi_long arg2)
+{
+return get_errno(setreuid(arg1, arg2));
+}
+
+/* setregid(2) */
+static inline abi_long do_bsd_setregid(abi_long arg1, abi_long arg2)
+{
+return get_errno(setregid(arg1, arg2));
+}
+
 #endif /* !BSD_PROC_H_ */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 5cb6086230..7565e69e76 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -255,6 +255,66 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_bsd_setrlimit(arg1, arg2);
 break;
 
+case TARGET_FREEBSD_NR_getpid: /* getpid(2) */
+ret = do_bsd_getpid();
+break;
+
+case TARGET_FREEBSD_NR_getppid: /* getppid(2) */
+ret = do_bsd_getppid();
+break;
+
+case TARGET_FREEBSD_NR_getuid: /* getuid(2) */
+ret = do_bsd_getuid();
+break;
+
+case TARGET_FREEBSD_NR_geteuid: /* geteuid(2) */
+ret = do_bsd_geteuid();
+break;
+
+case TARGET_FREEBSD_NR_getgid: /* getgid(2) */
+ret = do_bsd_getgid();
+break;
+
+case TARGET_FREEBSD_NR_getegid: /* getegid(2) */
+ret = do_bsd_getegid();
+break;
+
+case TARGET_FREEBSD_NR_setuid: /* setuid(2) */
+ret = do_bsd_setuid(arg1);
+break;
+
+case TARGET_FREEBSD_NR_seteuid: /* seteuid(2) */
+ret = do_bsd_seteuid(arg1);
+break;
+
+case TARGET_FREEBSD_NR_setgid: /* setgid(2) */
+ret = do_bsd_setgid(arg1);
+break;
+
+case TARGET_FREEBSD_NR_setegid: /* setegid(2) */
+ret = do_bsd_setegid(arg1);
+break;
+
+case TARGET_FREEBSD_NR_getpgrp: /* getpgrp(2) */
+ret = do_bsd_getpgrp();
+break;
+
+case TARGET_FREEBSD_NR_getpgid: /* getpgid(2) */
+ ret = do_bsd_getpgid(arg1);
+ break;
+
+case TARGET_FREEBSD_NR_setpgid: /* setpgid(2) */
+ ret = do_bsd_setpgid(arg1, arg2);
+ break;
+
+case TARGET_FREEBSD_NR_setreuid: /* setreuid(2) */
+ret = do_bsd_setreuid(arg1, arg2);
+break;
+
+case TARGET_FREEBSD_NR_setregid: /* setregid(2) */
+ret = do_bsd_setregid(arg1, arg2);
+break;
+
 
 /*
  * File system calls.
-- 
2.42.0

[PATCH v4 19/28] bsd-user: Implement get_filename_from_fd.

[Karim Taha]

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/freebsd/meson.build |  1 +
 bsd-user/freebsd/os-proc.c   | 80 
 2 files changed, 81 insertions(+)
 create mode 100644 bsd-user/freebsd/os-proc.c

diff --git a/bsd-user/freebsd/meson.build b/bsd-user/freebsd/meson.build
index f2f047cca3..8fd6c7cfb8 100644
--- a/bsd-user/freebsd/meson.build
+++ b/bsd-user/freebsd/meson.build
@@ -1,5 +1,6 @@
 bsd_user_ss.add(files(
   'os-stat.c',
+  'os-proc.c',
   'os-sys.c',
   'os-syscall.c',
 ))
diff --git a/bsd-user/freebsd/os-proc.c b/bsd-user/freebsd/os-proc.c
new file mode 100644
index 00..cb35f29f10
--- /dev/null
+++ b/bsd-user/freebsd/os-proc.c
@@ -0,0 +1,80 @@
+/*
+ *  FreeBSD process related emulation code
+ *
+ *  Copyright (c) 2013-15 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+#include "qemu/osdep.h"
+
+#include 
+#include 
+#include 
+struct kinfo_proc;
+#include 
+
+#include "qemu.h"
+
+/*
+ * Get the filename for the given file descriptor.
+ * Note that this may return NULL (fail) if no longer cached in the kernel.
+ */
+static char *
+get_filename_from_fd(pid_t pid, int fd, char *filename, size_t len)
+{
+char *ret = NULL;
+unsigned int cnt;
+struct procstat *procstat = NULL;
+struct kinfo_proc *kp = NULL;
+struct filestat_list *head = NULL;
+struct filestat *fst;
+
+procstat = procstat_open_sysctl();
+if (procstat == NULL) {
+goto out;
+}
+
+kp = procstat_getprocs(procstat, KERN_PROC_PID, pid, &cnt);
+if (kp == NULL) {
+goto out;
+}
+
+head = procstat_getfiles(procstat, kp, 0);
+if (head == NULL) {
+goto out;
+}
+
+STAILQ_FOREACH(fst, head, next) {
+if (fd == fst->fs_fd) {
+if (fst->fs_path != NULL) {
+(void)strlcpy(filename, fst->fs_path, len);
+ret = filename;
+}
+break;
+}
+}
+
+out:
+if (head != NULL) {
+procstat_freefiles(procstat, head);
+}
+if (kp != NULL) {
+procstat_freeprocs(procstat, kp);
+}
+if (procstat != NULL) {
+procstat_close(procstat);
+}
+return ret;
+}
+
-- 
2.42.0

[PATCH v4 06/28] bsd-user: Implement target_to_host_resource conversion function

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
---
 bsd-user/bsd-proc.c  | 40 
 bsd-user/bsd-proc.h  |  4 
 bsd-user/meson.build |  6 ++
 3 files changed, 50 insertions(+)
 create mode 100644 bsd-user/bsd-proc.c

diff --git a/bsd-user/bsd-proc.c b/bsd-user/bsd-proc.c
new file mode 100644
index 00..68410a0aa9
--- /dev/null
+++ b/bsd-user/bsd-proc.c
@@ -0,0 +1,40 @@
+/*
+ *  BSD process related system call helpers
+ *
+ *  Copyright (c) 2013-14 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+#include "qemu/osdep.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "qemu.h"
+#include "qemu-bsd.h"
+#include "signal-common.h"
+
+#include "bsd-proc.h"
+
+/*
+ * resource/rusage conversion
+ */
+int target_to_host_resource(int code)
+{
+return code;
+}
+
diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
index a1061bffb8..048773a75d 100644
--- a/bsd-user/bsd-proc.h
+++ b/bsd-user/bsd-proc.h
@@ -22,6 +22,10 @@
 
 #include 
 
+#include "qemu-bsd.h"
+#include "gdbstub/syscalls.h"
+#include "qemu/plugin.h"
+
 /* exit(2) */
 static inline abi_long do_bsd_exit(void *cpu_env, abi_long arg1)
 {
diff --git a/bsd-user/meson.build b/bsd-user/meson.build
index 5243122fc5..b97fce1472 100644
--- a/bsd-user/meson.build
+++ b/bsd-user/meson.build
@@ -7,6 +7,7 @@ bsd_user_ss = ss.source_set()
 common_user_inc += include_directories('include')
 
 bsd_user_ss.add(files(
+  'bsd-proc.c',
   'bsdload.c',
   'elfload.c',
   'main.c',
@@ -16,6 +17,11 @@ bsd_user_ss.add(files(
   'uaccess.c',
 ))
 
+elf = cc.find_library('elf', required: true)
+procstat = cc.find_library('procstat', required: true)
+kvm = cc.find_library('kvm', required: true)
+bsd_user_ss.add(elf, procstat, kvm)
+
 # Pull in the OS-specific build glue, if any
 subdir(targetos)
 
-- 
2.42.0

[PATCH v4 14/28] bsd-user: Implement getrlimit(2) and setrlimit(2)

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/bsd-proc.h   | 59 +++
 bsd-user/freebsd/os-syscall.c |  8 +
 2 files changed, 67 insertions(+)

diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
index 133c1b0eaf..38d1324034 100644
--- a/bsd-user/bsd-proc.h
+++ b/bsd-user/bsd-proc.h
@@ -137,4 +137,63 @@ static inline abi_long do_bsd_getrusage(abi_long who, 
abi_ulong target_addr)
 return ret;
 }
 
+/* getrlimit(2) */
+static inline abi_long do_bsd_getrlimit(abi_long arg1, abi_ulong arg2)
+{
+abi_long ret;
+int resource = target_to_host_resource(arg1);
+struct target_rlimit *target_rlim;
+struct rlimit rlim;
+
+switch (resource) {
+case RLIMIT_STACK:
+rlim.rlim_cur = target_dflssiz;
+rlim.rlim_max = target_maxssiz;
+ret = 0;
+break;
+
+case RLIMIT_DATA:
+rlim.rlim_cur = target_dfldsiz;
+rlim.rlim_max = target_maxdsiz;
+ret = 0;
+break;
+
+default:
+ret = get_errno(getrlimit(resource, &rlim));
+break;
+}
+if (!is_error(ret)) {
+if (!lock_user_struct(VERIFY_WRITE, target_rlim, arg2, 0)) {
+return -TARGET_EFAULT;
+}
+target_rlim->rlim_cur = host_to_target_rlim(rlim.rlim_cur);
+target_rlim->rlim_max = host_to_target_rlim(rlim.rlim_max);
+unlock_user_struct(target_rlim, arg2, 1);
+}
+return ret;
+}
+
+/* setrlimit(2) */
+static inline abi_long do_bsd_setrlimit(abi_long arg1, abi_ulong arg2)
+{
+abi_long ret;
+int resource = target_to_host_resource(arg1);
+struct target_rlimit *target_rlim;
+struct rlimit rlim;
+
+if (RLIMIT_STACK == resource) {
+/* XXX We should, maybe, allow the stack size to shrink */
+ret = -TARGET_EPERM;
+} else {
+if (!lock_user_struct(VERIFY_READ, target_rlim, arg2, 1)) {
+return -TARGET_EFAULT;
+}
+rlim.rlim_cur = target_to_host_rlim(target_rlim->rlim_cur);
+rlim.rlim_max = target_to_host_rlim(target_rlim->rlim_max);
+unlock_user_struct(target_rlim, arg2, 0);
+ret = get_errno(setrlimit(resource, &rlim));
+}
+return ret;
+}
+
 #endif /* !BSD_PROC_H_ */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 5d8693ed55..5cb6086230 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -247,6 +247,14 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_bsd_getrusage(arg1, arg2);
 break;
 
+case TARGET_FREEBSD_NR_getrlimit: /* getrlimit(2) */
+ret = do_bsd_getrlimit(arg1, arg2);
+break;
+
+case TARGET_FREEBSD_NR_setrlimit: /* setrlimit(2) */
+ret = do_bsd_setrlimit(arg1, arg2);
+break;
+
 
 /*
  * File system calls.
-- 
2.42.0

[PATCH v4 09/28] bsd-user: Implement host_to_target_waitstatus conversion.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/bsd-proc.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/bsd-user/bsd-proc.c b/bsd-user/bsd-proc.c
index aa386ff482..19f6efe1f7 100644
--- a/bsd-user/bsd-proc.c
+++ b/bsd-user/bsd-proc.c
@@ -102,3 +102,20 @@ abi_long host_to_target_wrusage(abi_ulong target_addr,
 return 0;
 }
 
+/*
+ * wait status conversion.
+ *
+ * Map host to target signal numbers for the wait family of syscalls.
+ * Assume all other status bits are the same.
+ */
+int host_to_target_waitstatus(int status)
+{
+if (WIFSIGNALED(status)) {
+return host_to_target_signal(WTERMSIG(status)) | (status & ~0x7f);
+}
+if (WIFSTOPPED(status)) {
+return (host_to_target_signal(WSTOPSIG(status)) << 8) | (status & 
0xff);
+}
+return status;
+}
+
-- 
2.42.0

[PATCH v4 20/28] bsd-user: Implement freebsd_exec_common, used in implementing execve/fexecve.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Warner Losh 
---
 bsd-user/freebsd/os-proc.c | 177 +
 bsd-user/main.c|   2 +-
 bsd-user/qemu.h|   1 +
 3 files changed, 179 insertions(+), 1 deletion(-)

diff --git a/bsd-user/freebsd/os-proc.c b/bsd-user/freebsd/os-proc.c
index cb35f29f10..12d78b7fc9 100644
--- a/bsd-user/freebsd/os-proc.c
+++ b/bsd-user/freebsd/os-proc.c
@@ -78,3 +78,180 @@ out:
 return ret;
 }
 
+/*
+ * execve/fexecve
+ */
+abi_long freebsd_exec_common(abi_ulong path_or_fd, abi_ulong guest_argp,
+abi_ulong guest_envp, int do_fexec)
+{
+char **argp, **envp, **qargp, **qarg1, **qarg0, **qargend;
+int argc, envc;
+abi_ulong gp;
+abi_ulong addr;
+char **q;
+int total_size = 0;
+void *p;
+abi_long ret;
+
+argc = 0;
+for (gp = guest_argp; gp; gp += sizeof(abi_ulong)) {
+if (get_user_ual(addr, gp)) {
+return -TARGET_EFAULT;
+}
+if (!addr) {
+break;
+}
+argc++;
+}
+envc = 0;
+for (gp = guest_envp; gp; gp += sizeof(abi_ulong)) {
+if (get_user_ual(addr, gp)) {
+return -TARGET_EFAULT;
+}
+if (!addr) {
+break;
+}
+envc++;
+}
+
+qarg0 = argp = g_new0(char *, argc + 9);
+/* save the first agrument for the emulator */
+*argp++ = (char *)getprogname();
+qargp = argp;
+*argp++ = (char *)getprogname();
+qarg1 = argp;
+envp = g_new0(char *, envc + 1);
+for (gp = guest_argp, q = argp; gp; gp += sizeof(abi_ulong), q++) {
+if (get_user_ual(addr, gp)) {
+ret = -TARGET_EFAULT;
+goto execve_end;
+}
+if (!addr) {
+break;
+}
+*q = lock_user_string(addr);
+if (*q == NULL) {
+ret = -TARGET_EFAULT;
+goto execve_end;
+}
+total_size += strlen(*q) + 1;
+}
+*q++ = NULL;
+qargend = q;
+
+for (gp = guest_envp, q = envp; gp; gp += sizeof(abi_ulong), q++) {
+if (get_user_ual(addr, gp)) {
+ret = -TARGET_EFAULT;
+goto execve_end;
+}
+if (!addr) {
+break;
+}
+*q = lock_user_string(addr);
+if (*q == NULL) {
+ret = -TARGET_EFAULT;
+goto execve_end;
+}
+total_size += strlen(*q) + 1;
+}
+*q = NULL;
+
+/*
+ * This case will not be caught by the host's execve() if its
+ * page size is bigger than the target's.
+ */
+if (total_size > MAX_ARG_PAGES * TARGET_PAGE_SIZE) {
+ret = -TARGET_E2BIG;
+goto execve_end;
+}
+
+if (do_fexec) {
+if (((int)path_or_fd > 0 &&
+is_target_elf_binary((int)path_or_fd)) == 1) {
+char execpath[PATH_MAX];
+
+/*
+ * The executable is an elf binary for the target
+ * arch.  execve() it using the emulator if we can
+ * determine the filename path from the fd.
+ */
+if (get_filename_from_fd(getpid(), (int)path_or_fd, execpath,
+sizeof(execpath)) != NULL) {
+memmove(qarg1 + 2, qarg1, (qargend - qarg1) * sizeof(*qarg1));
+qarg1[1] = qarg1[0];
+qarg1[0] = (char *)"-0";
+qarg1 += 2;
+qargend += 2;
+*qarg1 = execpath;
+#ifndef DONT_INHERIT_INTERP_PREFIX
+memmove(qarg1 + 2, qarg1, (qargend - qarg1) * sizeof(*qarg1));
+*qarg1++ = (char *)"-L";
+*qarg1++ = (char *)interp_prefix;
+#endif
+ret = get_errno(execve(qemu_proc_pathname, qargp, envp));
+} else {
+/* Getting the filename path failed. */
+ret = -TARGET_EBADF;
+goto execve_end;
+}
+} else {
+ret = get_errno(fexecve((int)path_or_fd, argp, envp));
+}
+} else {
+int fd;
+
+p = lock_user_string(path_or_fd);
+if (p == NULL) {
+ret = -TARGET_EFAULT;
+goto execve_end;
+}
+
+/*
+ * Check the header and see if it a target elf binary.  If so
+ * then execute using qemu user mode emulator.
+ */
+fd = open(p, O_RDONLY | O_CLOEXEC);
+if (fd > 0 && is_target_elf_binary(fd) == 1) {
+close(fd);
+/* execve() as a target binary using emulator. */
+memmove(qarg1 + 2, qarg1, (qargend - qarg1) * sizeof(*qarg1));
+qarg1[1] = qarg1[0];
+qarg1[0] = (char *)"-0";
+qarg1 += 2;
+qargend += 2;
+*qarg1 = (char *)p;
+#ifndef DONT_INHERIT_INTERP_PREFIX
+memmove(qarg1 + 2, qarg1, (qargend - qarg1) * sizeof(*qarg1));
+*qarg1++ = (char *)"-L";
+*qarg1++ =

[PATCH v4 22/28] bsd-user: Implement execve(2) and fexecve(2) system calls.

[Karim Taha]

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/freebsd/os-proc.h| 49 +++
 bsd-user/freebsd/os-syscall.c | 11 +++-
 2 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 bsd-user/freebsd/os-proc.h

diff --git a/bsd-user/freebsd/os-proc.h b/bsd-user/freebsd/os-proc.h
new file mode 100644
index 00..75ed39f8dd
--- /dev/null
+++ b/bsd-user/freebsd/os-proc.h
@@ -0,0 +1,49 @@
+/*
+ *  process related system call shims and definitions
+ *
+ *  Copyright (c) 2013-14 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef BSD_USER_FREEBSD_OS_PROC_H
+#define BSD_USER_FREEBSD_OS_PROC_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "target_arch_cpu.h"
+
+/* execve(2) */
+static inline abi_long do_freebsd_execve(abi_ulong path_or_fd, abi_ulong argp,
+abi_ulong envp)
+{
+
+return freebsd_exec_common(path_or_fd, argp, envp, 0);
+}
+
+/* fexecve(2) */
+static inline abi_long do_freebsd_fexecve(abi_ulong path_or_fd, abi_ulong argp,
+abi_ulong envp)
+{
+
+return freebsd_exec_common(path_or_fd, argp, envp, 1);
+}
+
+#endif /* BSD_USER_FREEBSD_OS_PROC_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index b7bd0b92a6..515eaaf31f 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -36,8 +36,9 @@
 #include "bsd-file.h"
 #include "bsd-proc.h"
 
-/* *BSD dependent syscall shims */
+/* BSD dependent syscall shims */
 #include "os-stat.h"
+#include "os-proc.h"
 
 /* I/O */
 safe_syscall3(int, open, const char *, path, int, flags, mode_t, mode);
@@ -219,6 +220,14 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 /*
  * process system calls
  */
+case TARGET_FREEBSD_NR_execve: /* execve(2) */
+ret = do_freebsd_execve(arg1, arg2, arg3);
+break;
+
+case TARGET_FREEBSD_NR_fexecve: /* fexecve(2) */
+ret = do_freebsd_fexecve(arg1, arg2, arg3);
+break;
+
 case TARGET_FREEBSD_NR_exit: /* exit(2) */
 ret = do_bsd_exit(cpu_env, arg1);
 break;
-- 
2.42.0

[PATCH v4 17/28] bsd-user: Add stubs for profil(2), ktrace(2), utrace(2) and ptrace(2).

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/bsd-proc.h   | 28 
 bsd-user/freebsd/os-syscall.c | 16 
 2 files changed, 44 insertions(+)

diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
index a5f301c72f..2c1a9ae22f 100644
--- a/bsd-user/bsd-proc.h
+++ b/bsd-user/bsd-proc.h
@@ -362,4 +362,32 @@ static inline abi_long do_bsd_issetugid(void)
 return get_errno(issetugid());
 }
 
+/* profil(2) */
+static inline abi_long do_bsd_profil(abi_long arg1, abi_long arg2,
+ abi_long arg3, abi_long arg4)
+{
+return -TARGET_ENOSYS;
+}
+
+/* ktrace(2) */
+static inline abi_long do_bsd_ktrace(abi_long arg1, abi_long arg2,
+ abi_long arg3, abi_long arg4)
+{
+return -TARGET_ENOSYS;
+}
+
+/* utrace(2) */
+static inline abi_long do_bsd_utrace(abi_long arg1, abi_long arg2)
+{
+return -TARGET_ENOSYS;
+}
+
+
+/* ptrace(2) */
+static inline abi_long do_bsd_ptrace(abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4)
+{
+return -TARGET_ENOSYS;
+}
+
 #endif /* !BSD_PROC_H_ */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 7b51f4f16e..1a760b1380 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -343,6 +343,22 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_bsd_issetugid();
 break;
 
+case TARGET_FREEBSD_NR_profil: /* profil(2) */
+ret = do_bsd_profil(arg1, arg2, arg3, arg4);
+break;
+
+case TARGET_FREEBSD_NR_ktrace: /* ktrace(2) */
+ret = do_bsd_ktrace(arg1, arg2, arg3, arg4);
+break;
+
+case TARGET_FREEBSD_NR_utrace: /* utrace(2) */
+ret = do_bsd_utrace(arg1, arg2);
+break;
+
+case TARGET_FREEBSD_NR_ptrace: /* ptrace(2) */
+ret = do_bsd_ptrace(arg1, arg2, arg3, arg4);
+break;
+
 
 /*
  * File system calls.
-- 
2.42.0

[PATCH v4 18/28] bsd-user: Implement getpriority(2) and setpriority(2).

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Warner Losh 
---
 bsd-user/bsd-proc.h   | 24 
 bsd-user/freebsd/os-syscall.c |  8 
 2 files changed, 32 insertions(+)

diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
index 2c1a9ae22f..9a8912361f 100644
--- a/bsd-user/bsd-proc.h
+++ b/bsd-user/bsd-proc.h
@@ -390,4 +390,28 @@ static inline abi_long do_bsd_ptrace(abi_long arg1, 
abi_long arg2,
 return -TARGET_ENOSYS;
 }
 
+/* getpriority(2) */
+static inline abi_long do_bsd_getpriority(abi_long which, abi_long who)
+{
+abi_long ret;
+/*
+ * Note that negative values are valid for getpriority, so we must
+ * differentiate based on errno settings.
+ */
+errno = 0;
+ret = getpriority(which, who);
+if (ret == -1 && errno != 0) {
+return -host_to_target_errno(errno);
+}
+
+return ret;
+}
+
+/* setpriority(2) */
+static inline abi_long do_bsd_setpriority(abi_long which, abi_long who,
+  abi_long prio)
+{
+return get_errno(setpriority(which, who, prio));
+}
+
 #endif /* !BSD_PROC_H_ */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 1a760b1380..71a2657dd0 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -359,6 +359,14 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_bsd_ptrace(arg1, arg2, arg3, arg4);
 break;
 
+case TARGET_FREEBSD_NR_getpriority: /* getpriority(2) */
+ret = do_bsd_getpriority(arg1, arg2);
+break;
+
+case TARGET_FREEBSD_NR_setpriority: /* setpriority(2) */
+ret = do_bsd_setpriority(arg1, arg2, arg3);
+break;
+
 
 /*
  * File system calls.
-- 
2.42.0

[PATCH v4 28/28] bsd-user: Implement pdfork(2) system call.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Acked-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/freebsd/os-proc.h| 32 
 bsd-user/freebsd/os-syscall.c |  4 
 2 files changed, 36 insertions(+)

diff --git a/bsd-user/freebsd/os-proc.h b/bsd-user/freebsd/os-proc.h
index 0a3cd0ef57..d641878034 100644
--- a/bsd-user/freebsd/os-proc.h
+++ b/bsd-user/freebsd/os-proc.h
@@ -258,4 +258,36 @@ static inline abi_long do_freebsd_rfork(void *cpu_env, 
abi_long flags)
 
 }
 
+/* pdfork(2) */
+static inline abi_long do_freebsd_pdfork(void *cpu_env, abi_ulong target_fdp,
+abi_long flags)
+{
+abi_long ret;
+abi_ulong child_flag;
+int fd;
+
+fork_start();
+ret = pdfork(&fd, flags);
+if (ret == 0) {
+/* child */
+child_flag = 1;
+target_cpu_clone_regs(cpu_env, 0);
+} else {
+/* parent */
+child_flag = 0;
+if (put_user_s32(fd, target_fdp)) {
+return -TARGET_EFAULT;
+}
+}
+
+/*
+ * The fork system call sets a child flag in the second return
+ * value: 0 for parent process, 1 for child process.
+ */
+set_second_rval(cpu_env, child_flag);
+fork_end(child_flag);
+
+return ret;
+}
+
 #endif /* BSD_USER_FREEBSD_OS_PROC_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 4c4e773d1d..d04712f0a7 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -238,6 +238,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_freebsd_rfork(cpu_env, arg1);
 break;
 
+case TARGET_FREEBSD_NR_pdfork: /* pdfork(2) */
+ret = do_freebsd_pdfork(cpu_env, arg1, arg2);
+break;
+
 case TARGET_FREEBSD_NR_execve: /* execve(2) */
 ret = do_freebsd_execve(arg1, arg2, arg3);
 break;
-- 
2.42.0

[PATCH v4 24/28] bsd-user: Implement setloginclass(2) and getloginclass(2) system calls.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
---
 bsd-user/freebsd/os-proc.h| 32 
 bsd-user/freebsd/os-syscall.c |  8 
 2 files changed, 40 insertions(+)

diff --git a/bsd-user/freebsd/os-proc.h b/bsd-user/freebsd/os-proc.h
index 04bce755e5..2eaba141dc 100644
--- a/bsd-user/freebsd/os-proc.h
+++ b/bsd-user/freebsd/os-proc.h
@@ -130,4 +130,36 @@ static inline abi_long do_freebsd_wait6(void *cpu_env, 
abi_long idtype,
 return ret;
 }
 
+/* setloginclass(2) */
+static inline abi_long do_freebsd_setloginclass(abi_ulong arg1)
+{
+abi_long ret;
+void *p;
+
+p = lock_user_string(arg1);
+if (p == NULL) {
+return -TARGET_EFAULT;
+}
+ret = get_errno(setloginclass(p));
+unlock_user(p, arg1, 0);
+
+return ret;
+}
+
+/* getloginclass(2) */
+static inline abi_long do_freebsd_getloginclass(abi_ulong arg1, abi_ulong arg2)
+{
+abi_long ret;
+void *p;
+
+p = lock_user(VERIFY_WRITE, arg1, arg2, 0);
+if (p == NULL) {
+return -TARGET_EFAULT;
+}
+ret = get_errno(getloginclass(p, arg2));
+unlock_user(p, arg1, arg2);
+
+return ret;
+}
+
 #endif /* BSD_USER_FREEBSD_OS_PROC_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 55e68e4815..d614409e69 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -375,6 +375,14 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_bsd_ktrace(arg1, arg2, arg3, arg4);
 break;
 
+case TARGET_FREEBSD_NR_setloginclass: /* setloginclass(2) */
+ret = do_freebsd_setloginclass(arg1);
+break;
+
+case TARGET_FREEBSD_NR_getloginclass: /* getloginclass(2) */
+ret = do_freebsd_getloginclass(arg1, arg2);
+break;
+
 case TARGET_FREEBSD_NR_utrace: /* utrace(2) */
 ret = do_bsd_utrace(arg1, arg2);
 break;
-- 
2.42.0

[PATCH v4 21/28] bsd-user: Implement procctl(2) along with necessary conversion functions.

[Karim Taha]

From: Stacey Son 

Implement t2h_procctl_cmd, h2t_reaper_status, h2t_reaper_pidinfo and h2t/t2h 
reaper_kill conversion functions.

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
---
 bsd-user/freebsd/os-proc.c| 222 ++
 bsd-user/freebsd/os-syscall.c |   3 +
 2 files changed, 225 insertions(+)

diff --git a/bsd-user/freebsd/os-proc.c b/bsd-user/freebsd/os-proc.c
index 12d78b7fc9..6c0c7a9d67 100644
--- a/bsd-user/freebsd/os-proc.c
+++ b/bsd-user/freebsd/os-proc.c
@@ -255,3 +255,225 @@ execve_end:
 return ret;
 }
 
+#include 
+
+static abi_long
+t2h_procctl_cmd(int target_cmd, int *host_cmd)
+{
+switch (target_cmd) {
+case TARGET_PROC_SPROTECT:
+*host_cmd = PROC_SPROTECT;
+break;
+
+case TARGET_PROC_REAP_ACQUIRE:
+*host_cmd = PROC_REAP_ACQUIRE;
+break;
+
+case TARGET_PROC_REAP_RELEASE:
+*host_cmd = PROC_REAP_RELEASE;
+break;
+
+case TARGET_PROC_REAP_STATUS:
+*host_cmd = PROC_REAP_STATUS;
+break;
+
+case TARGET_PROC_REAP_KILL:
+*host_cmd = PROC_REAP_KILL;
+break;
+
+default:
+return -TARGET_EINVAL;
+}
+
+return 0;
+}
+
+static abi_long
+h2t_reaper_status(struct procctl_reaper_status *host_rs,
+abi_ulong target_rs_addr)
+{
+struct target_procctl_reaper_status *target_rs;
+
+if (!lock_user_struct(VERIFY_WRITE, target_rs, target_rs_addr, 0)) {
+return -TARGET_EFAULT;
+}
+__put_user(host_rs->rs_flags, &target_rs->rs_flags);
+__put_user(host_rs->rs_children, &target_rs->rs_children);
+__put_user(host_rs->rs_descendants, &target_rs->rs_descendants);
+__put_user(host_rs->rs_reaper, &target_rs->rs_reaper);
+__put_user(host_rs->rs_pid, &target_rs->rs_pid);
+unlock_user_struct(target_rs, target_rs_addr, 1);
+
+return 0;
+}
+
+static abi_long
+t2h_reaper_kill(abi_ulong target_rk_addr, struct procctl_reaper_kill *host_rk)
+{
+struct target_procctl_reaper_kill *target_rk;
+
+if (!lock_user_struct(VERIFY_READ, target_rk, target_rk_addr, 1)) {
+return -TARGET_EFAULT;
+}
+__get_user(host_rk->rk_sig, &target_rk->rk_sig);
+__get_user(host_rk->rk_flags, &target_rk->rk_flags);
+__get_user(host_rk->rk_subtree, &target_rk->rk_subtree);
+__get_user(host_rk->rk_killed, &target_rk->rk_killed);
+__get_user(host_rk->rk_fpid, &target_rk->rk_fpid);
+unlock_user_struct(target_rk, target_rk_addr, 0);
+
+return 0;
+}
+
+static abi_long
+h2t_reaper_kill(struct procctl_reaper_kill *host_rk, abi_ulong target_rk_addr)
+{
+struct target_procctl_reaper_kill *target_rk;
+
+if (!lock_user_struct(VERIFY_WRITE, target_rk, target_rk_addr, 0)) {
+return -TARGET_EFAULT;
+}
+__put_user(host_rk->rk_sig, &target_rk->rk_sig);
+__put_user(host_rk->rk_flags, &target_rk->rk_flags);
+__put_user(host_rk->rk_subtree, &target_rk->rk_subtree);
+__put_user(host_rk->rk_killed, &target_rk->rk_killed);
+__put_user(host_rk->rk_fpid, &target_rk->rk_fpid);
+unlock_user_struct(target_rk, target_rk_addr, 1);
+
+return 0;
+}
+
+static abi_long
+h2t_procctl_reaper_pidinfo(struct procctl_reaper_pidinfo *host_pi,
+abi_ulong target_pi_addr)
+{
+struct target_procctl_reaper_pidinfo *target_pi;
+
+if (!lock_user_struct(VERIFY_WRITE, target_pi, target_pi_addr, 0)) {
+return -TARGET_EFAULT;
+}
+__put_user(host_pi->pi_pid, &target_pi->pi_pid);
+__put_user(host_pi->pi_subtree, &target_pi->pi_subtree);
+__put_user(host_pi->pi_flags, &target_pi->pi_flags);
+unlock_user_struct(target_pi, target_pi_addr, 1);
+
+return 0;
+}
+
+abi_long
+do_freebsd_procctl(void *cpu_env, int idtype, abi_ulong arg2, abi_ulong arg3,
+   abi_ulong arg4, abi_ulong arg5, abi_ulong arg6)
+{
+abi_long error = 0, target_rp_pids;
+void *data;
+int host_cmd, flags;
+uint32_t u, target_rp_count;
+g_autofree union {
+struct procctl_reaper_status rs;
+struct procctl_reaper_pids rp;
+struct procctl_reaper_kill rk;
+} host;
+struct target_procctl_reaper_pids *target_rp;
+id_t id; /* 64-bit */
+int target_cmd;
+abi_ulong target_arg;
+
+#if TARGET_ABI_BITS == 32
+/* See if we need to align the register pairs. */
+if (regpairs_aligned(cpu_env)) {
+id = (id_t)target_arg64(arg3, arg4);
+target_cmd = (int)arg5;
+target_arg = arg6;
+} else {
+id = (id_t)target_arg64(arg2, arg3);
+target_cmd = (int)arg4;
+target_arg = arg5;
+}
+#else
+id = (id_t)arg2;
+target_cmd = (int)arg3;
+target_arg = arg4;
+#endif
+
+error = t2h_procctl_cmd(target_cmd, &host_cmd);
+if (error) {
+return error;
+}
+switch (host_cmd) {
+case PROC_SPROTECT:
+data = &flags;
+break;
+
+case PROC_REAP_ACQUIRE:
+case PROC_REAP_RELEASE:
+if (target_arg == 0) {
+data = NULL;
+} else 

[PATCH v4 11/28] bsd-user: Implement getgroups(2) and setgroups(2) system calls.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Warner Losh 
---
 bsd-user/bsd-proc.h   | 44 +++
 bsd-user/freebsd/os-syscall.c |  9 +++
 2 files changed, 53 insertions(+)

diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
index b6225e520e..7b25aa1982 100644
--- a/bsd-user/bsd-proc.h
+++ b/bsd-user/bsd-proc.h
@@ -41,4 +41,48 @@ static inline abi_long do_bsd_exit(void *cpu_env, abi_long 
arg1)
 return 0;
 }
 
+/* getgroups(2) */
+static inline abi_long do_bsd_getgroups(abi_long gidsetsize, abi_long arg2)
+{
+abi_long ret;
+uint32_t *target_grouplist;
+g_autofree gid_t *grouplist;
+int i;
+
+grouplist = g_try_new(gid_t, gidsetsize);
+ret = get_errno(getgroups(gidsetsize, grouplist));
+if (gidsetsize != 0) {
+if (!is_error(ret)) {
+target_grouplist = lock_user(VERIFY_WRITE, arg2, gidsetsize * 2, 
0);
+if (!target_grouplist) {
+return -TARGET_EFAULT;
+}
+for (i = 0; i < ret; i++) {
+target_grouplist[i] = tswap32(grouplist[i]);
+}
+unlock_user(target_grouplist, arg2, gidsetsize * 2);
+}
+}
+return ret;
+}
+
+/* setgroups(2) */
+static inline abi_long do_bsd_setgroups(abi_long gidsetsize, abi_long arg2)
+{
+uint32_t *target_grouplist;
+g_autofree gid_t *grouplist;
+int i;
+
+grouplist = g_try_new(gid_t, gidsetsize);
+target_grouplist = lock_user(VERIFY_READ, arg2, gidsetsize * 2, 1);
+if (!target_grouplist) {
+return -TARGET_EFAULT;
+}
+for (i = 0; i < gidsetsize; i++) {
+grouplist[i] = tswap32(target_grouplist[i]);
+}
+unlock_user(target_grouplist, arg2, 0);
+return get_errno(setgroups(gidsetsize, grouplist));
+}
+
 #endif /* !BSD_PROC_H_ */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index fa60df529e..535e6287bd 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -223,6 +223,15 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_bsd_exit(cpu_env, arg1);
 break;
 
+case TARGET_FREEBSD_NR_getgroups: /* getgroups(2) */
+ret = do_bsd_getgroups(arg1, arg2);
+break;
+
+case TARGET_FREEBSD_NR_setgroups: /* setgroups(2) */
+ret = do_bsd_setgroups(arg1, arg2);
+break;
+
+
 /*
  * File system calls.
  */
-- 
2.42.0

[PATCH v4 10/28] bsd-user: Get number of cpus.

[Karim Taha]

From: Kyle Evans 

Signed-off-by: Kyle Evans 
Signed-off-by: Karim Taha 
Reviewed-by: Warner Losh 
---
 bsd-user/bsd-proc.c | 24 
 bsd-user/bsd-proc.h |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/bsd-user/bsd-proc.c b/bsd-user/bsd-proc.c
index 19f6efe1f7..ca3c1bf94f 100644
--- a/bsd-user/bsd-proc.c
+++ b/bsd-user/bsd-proc.c
@@ -119,3 +119,27 @@ int host_to_target_waitstatus(int status)
 return status;
 }
 
+int bsd_get_ncpu(void)
+{
+int ncpu = -1;
+cpuset_t mask;
+
+CPU_ZERO(&mask);
+
+if (cpuset_getaffinity(CPU_LEVEL_WHICH, CPU_WHICH_TID, -1, sizeof(mask),
+   &mask) == 0) {
+ncpu = CPU_COUNT(&mask);
+}
+
+if (ncpu == -1) {
+ncpu = sysconf(_SC_NPROCESSORS_ONLN);
+}
+
+if (ncpu == -1) {
+gemu_log("XXX Missing bsd_get_ncpu() implementation\n");
+ncpu = 1;
+}
+
+return ncpu;
+}
+
diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
index 048773a75d..b6225e520e 100644
--- a/bsd-user/bsd-proc.h
+++ b/bsd-user/bsd-proc.h
@@ -26,6 +26,8 @@
 #include "gdbstub/syscalls.h"
 #include "qemu/plugin.h"
 
+int bsd_get_ncpu(void);
+
 /* exit(2) */
 static inline abi_long do_bsd_exit(void *cpu_env, abi_long arg1)
 {
-- 
2.42.0

[PATCH v4 27/28] bsd-user: Implement rfork(2) system call.

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
Reviewed-by: Richard Henderson 
Reviewed-by: Warner Losh 
---
 bsd-user/freebsd/os-proc.h| 39 +++
 bsd-user/freebsd/os-syscall.c |  4 
 2 files changed, 43 insertions(+)

diff --git a/bsd-user/freebsd/os-proc.h b/bsd-user/freebsd/os-proc.h
index 7b2e6a9f79..0a3cd0ef57 100644
--- a/bsd-user/freebsd/os-proc.h
+++ b/bsd-user/freebsd/os-proc.h
@@ -219,4 +219,43 @@ static inline abi_long do_freebsd_vfork(void *cpu_env)
 return do_freebsd_fork(cpu_env);
 }
 
+/* rfork(2) */
+static inline abi_long do_freebsd_rfork(void *cpu_env, abi_long flags)
+{
+abi_long ret;
+abi_ulong child_flag;
+
+/*
+ * XXX We need to handle RFMEM here, as well.  Neither are safe to execute
+ * as-is on x86 hosts because they'll split memory but not the stack,
+ * wreaking havoc on host architectures that use the stack to store the
+ * return address as both threads try to pop it off.  Rejecting RFSPAWN
+ * entirely for now is ok, the only consumer at the moment is posix_spawn
+ * and it will fall back to classic vfork(2) if we return EINVAL.
+ */
+if ((flags & TARGET_RFSPAWN) != 0) {
+return -TARGET_EINVAL;
+}
+fork_start();
+ret = rfork(flags);
+if (ret == 0) {
+/* child */
+child_flag = 1;
+target_cpu_clone_regs(cpu_env, 0);
+} else {
+/* parent */
+child_flag = 0;
+}
+
+/*
+ * The fork system call sets a child flag in the second return
+ * value: 0 for parent process, 1 for child process.
+ */
+set_second_rval(cpu_env, child_flag);
+fork_end(child_flag);
+
+return ret;
+
+}
+
 #endif /* BSD_USER_FREEBSD_OS_PROC_H */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index cb9425c9ba..4c4e773d1d 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -234,6 +234,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_freebsd_vfork(cpu_env);
 break;
 
+case TARGET_FREEBSD_NR_rfork: /* rfork(2) */
+ret = do_freebsd_rfork(cpu_env, arg1);
+break;
+
 case TARGET_FREEBSD_NR_execve: /* execve(2) */
 ret = do_freebsd_execve(arg1, arg2, arg3);
 break;
-- 
2.42.0

[PATCH v4 12/28] bsd-user: Implement umask(2), setlogin(2) and getlogin(2)

[Karim Taha]

From: Stacey Son 

Signed-off-by: Stacey Son 
Signed-off-by: Karim Taha 
---
 bsd-user/bsd-proc.h   | 39 +++
 bsd-user/freebsd/os-syscall.c | 12 +++
 2 files changed, 51 insertions(+)

diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
index 7b25aa1982..cb7c69acb0 100644
--- a/bsd-user/bsd-proc.h
+++ b/bsd-user/bsd-proc.h
@@ -26,6 +26,7 @@
 #include "gdbstub/syscalls.h"
 #include "qemu/plugin.h"
 
+extern int _getlogin(char*, int);
 int bsd_get_ncpu(void);
 
 /* exit(2) */
@@ -85,4 +86,42 @@ static inline abi_long do_bsd_setgroups(abi_long gidsetsize, 
abi_long arg2)
 return get_errno(setgroups(gidsetsize, grouplist));
 }
 
+/* umask(2) */
+static inline abi_long do_bsd_umask(abi_long arg1)
+{
+return get_errno(umask(arg1));
+}
+
+/* setlogin(2) */
+static inline abi_long do_bsd_setlogin(abi_long arg1)
+{
+abi_long ret;
+void *p;
+
+p = lock_user_string(arg1);
+if (p == NULL) {
+return -TARGET_EFAULT;
+}
+ret = get_errno(setlogin(p));
+unlock_user(p, arg1, 0);
+
+return ret;
+}
+
+/* getlogin(2) */
+static inline abi_long do_bsd_getlogin(abi_long arg1, abi_long arg2)
+{
+abi_long ret;
+void *p;
+
+p = lock_user(VERIFY_WRITE, arg1, arg2, 0);
+if (p == NULL) {
+return -TARGET_EFAULT;
+}
+ret = get_errno(_getlogin(p, arg2));
+unlock_user(p, arg1, arg2);
+
+return ret;
+}
+
 #endif /* !BSD_PROC_H_ */
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 535e6287bd..44cbf52f08 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -231,6 +231,18 @@ static abi_long freebsd_syscall(void *cpu_env, int num, 
abi_long arg1,
 ret = do_bsd_setgroups(arg1, arg2);
 break;
 
+case TARGET_FREEBSD_NR_umask: /* umask(2) */
+ret = do_bsd_umask(arg1);
+break;
+
+case TARGET_FREEBSD_NR_setlogin: /* setlogin(2) */
+ret = do_bsd_setlogin(arg1);
+break;
+
+case TARGET_FREEBSD_NR_getlogin: /* getlogin(2) */
+ret = do_bsd_getlogin(arg1, arg2);
+break;
+
 
 /*
  * File system calls.
-- 
2.42.0

Re: [PATCH v3 00/19] riscv: split TCG/KVM accelerators from cpu.c

[Alistair Francis]

On Wed, Sep 20, 2023 at 9:26 PM Daniel Henrique Barboza
 wrote:
>
> Hi,
>
> In this version we changed patch 10 (remove kvm-stub.c) as suggested by
> Phil to not include non-KVM stubs in kvm_riscv.h. A change in patch 05
> requested by Zhiwei was also made.
>
> Patches based on Alistair's riscv-to-apply.next.
>
> Patches missing acks: patch 10
>
> Changes from v2:
> - patch 05:
>   - remove riscv_cpu_add_user_properties() from riscv_host_cpu_init()
> - patch 10:
>   - do not add non-KVM stubs in kvm_riscv.h
> - v2 link: 
> https://lore.kernel.org/qemu-riscv/20230906091647.1667171-1-dbarb...@ventanamicro.com/
>
> Daniel Henrique Barboza (19):
>   target/riscv: introduce TCG AccelCPUClass
>   target/riscv: move riscv_cpu_realize_tcg() to TCG::cpu_realizefn()
>   target/riscv: move riscv_cpu_validate_set_extensions() to tcg-cpu.c
>   target/riscv: move riscv_tcg_ops to tcg-cpu.c
>   target/riscv/cpu.c: add .instance_post_init()
>   target/riscv: move 'host' CPU declaration to kvm.c
>   target/riscv/cpu.c: mark extensions arrays as 'const'
>   target/riscv: move riscv_cpu_add_kvm_properties() to kvm.c
>   target/riscv: make riscv_add_satp_mode_properties() public
>   target/riscv: remove kvm-stub.c
>   target/riscv: introduce KVM AccelCPUClass
>   target/riscv: move KVM only files to kvm subdir
>   target/riscv/kvm: do not use riscv_cpu_add_misa_properties()
>   target/riscv/cpu.c: export set_misa()
>   target/riscv/tcg: introduce tcg_cpu_instance_init()
>   target/riscv/cpu.c: make misa_ext_cfgs[] 'const'
>   target/riscv/tcg: move riscv_cpu_add_misa_properties() to tcg-cpu.c
>   target/riscv/cpu.c: export isa_edata_arr[]
>   target/riscv/cpu: move priv spec functions to tcg-cpu.c

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  hw/intc/riscv_aplic.c |   2 +-
>  hw/riscv/virt.c   |   2 +-
>  target/riscv/cpu.c| 988 ++
>  target/riscv/cpu.h|  30 +-
>  target/riscv/csr.c|   1 +
>  target/riscv/kvm-stub.c   |  30 -
>  target/riscv/{kvm.c => kvm/kvm-cpu.c} | 120 +++-
>  target/riscv/{ => kvm}/kvm_riscv.h|   4 -
>  target/riscv/kvm/meson.build  |   1 +
>  target/riscv/meson.build  |   4 +-
>  target/riscv/tcg/meson.build  |   2 +
>  target/riscv/tcg/tcg-cpu.c| 883 +++
>  target/riscv/tcg/tcg-cpu.h|  27 +
>  13 files changed, 1113 insertions(+), 981 deletions(-)
>  delete mode 100644 target/riscv/kvm-stub.c
>  rename target/riscv/{kvm.c => kvm/kvm-cpu.c} (91%)
>  rename target/riscv/{ => kvm}/kvm_riscv.h (89%)
>  create mode 100644 target/riscv/kvm/meson.build
>  create mode 100644 target/riscv/tcg/meson.build
>  create mode 100644 target/riscv/tcg/tcg-cpu.c
>  create mode 100644 target/riscv/tcg/tcg-cpu.h
>
> --
> 2.41.0
>
>

Re: [PATCH 21/52] migration/rdma: Fix QEMUFileHooks method return values

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> The QEMUFileHooks methods don't come with a written contract.  Digging
> through the code calling them, we find:
> 
> * save_page():

I'm fine with this

> 
>Negative values RAM_SAVE_CONTROL_DELAYED and
>RAM_SAVE_CONTROL_NOT_SUPP are special.  Any other negative value is
>an unspecified error.
> 
>qemu_rdma_save_page() returns -EIO or rdma->error_state on error.  I
>believe the latter is always negative.  Nothing stops either of them
>to clash with the special values, though.  Feels unlikely, but fix
>it anyway to return only the special values and -1.
> 
> * before_ram_iterate(), before_ram_iterate():

error code returned by before_ram_iterate() and before_ram_iterate() will be
assigned to QEMUFile for upper layer.
But it seems that no callers take care about the error ?  Shouldn't let the 
callers
to check the error instead ?



> 
>Negative value means error.  qemu_rdma_registration_start() and
>qemu_rdma_registration_stop() comply as far as I can tell.  Make
>them comply *obviously*, by returning -1 on error.
> 
> * hook_ram_load:
> 
>Negative value means error.  rdma_load_hook() already returns -1 on
>error.  Leave it alone.

see inline reply below,

> 
> Signed-off-by: Markus Armbruster 
> ---
>   migration/rdma.c | 79 +++-
>   1 file changed, 37 insertions(+), 42 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index cc59155a50..46b5859268 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3219,12 +3219,11 @@ static size_t qemu_rdma_save_page(QEMUFile *f,
>   rdma = qatomic_rcu_read(&rioc->rdmaout);
>   
>   if (!rdma) {
> -return -EIO;
> +return -1;
>   }
>   
> -ret = check_error_state(rdma);
> -if (ret) {
> -return ret;
> +if (check_error_state(rdma)) {
> +return -1;
>   }
>   
>   qemu_fflush(f);
> @@ -3290,9 +3289,10 @@ static size_t qemu_rdma_save_page(QEMUFile *f,
>   }
>   
>   return RAM_SAVE_CONTROL_DELAYED;
> +
>   err:
>   rdma->error_state = ret;
> -return ret;
> +return -1;
>   }
>   
>   static void rdma_accept_incoming_migration(void *opaque);
> @@ -3538,12 +3538,11 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
>   rdma = qatomic_rcu_read(&rioc->rdmain);
>   
>   if (!rdma) {
> -return -EIO;
> +return -1;

that's because EIO is not accurate here ?



>   }
>   
> -ret = check_error_state(rdma);
> -if (ret) {
> -return ret;

Ditto


Thanks
Zhijian

> +if (check_error_state(rdma)) {
> +return -1;
>   }
>   
>   local = &rdma->local_ram_blocks;
> @@ -3576,7 +3575,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
>(unsigned int)comp->block_idx,
>rdma->local_ram_blocks.nb_blocks);
>   ret = -EIO;
> -goto out;
> +goto err;
>   }
>   block = &(rdma->local_ram_blocks.block[comp->block_idx]);
>   
> @@ -3588,7 +3587,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
>   
>   case RDMA_CONTROL_REGISTER_FINISHED:
>   trace_qemu_rdma_registration_handle_finished();
> -goto out;
> +return 0;
>   
>   case RDMA_CONTROL_RAM_BLOCKS_REQUEST:
>   trace_qemu_rdma_registration_handle_ram_blocks();
> @@ -3609,7 +3608,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
>   if (ret) {
>   error_report("rdma migration: error dest "
>   "registering ram blocks");
> -goto out;
> +goto err;
>   }
>   }
>   
> @@ -3648,7 +3647,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
>   
>   if (ret < 0) {
>   error_report("rdma migration: error sending remote info");
> -goto out;
> +goto err;
>   }
>   
>   break;
> @@ -3675,7 +3674,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
>(unsigned int)reg->current_index,
>rdma->local_ram_blocks.nb_blocks);
>   ret = -ENOENT;
> -goto out;
> +goto err;
>   }
>   block = &(rdma->local_ram_blocks.block[reg->current_index]);
>   if (block->is_ram_block) {
> @@ -3685,7 +3684,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
>   block->block_name, block->offset,
>   reg->key.current_addr);
>   ret = -ERANGE;
> -goto out;
> +goto err;
>   }
>   h

[PATCH 2/4] target/riscv: cpu: Fixup local variables shadowing

[Alistair Francis]

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

--extra-cflags='-Wshadow=local -Wno-error=shadow=local 
-Wno-error=shadow=compatible-local'

To configure

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index eeeb08a35a..4dd1daada0 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -699,7 +699,7 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int 
flags)
 CSR_MPMMASK,
 };
 
-for (int i = 0; i < ARRAY_SIZE(dump_csrs); ++i) {
+for (i = 0; i < ARRAY_SIZE(dump_csrs); ++i) {
 int csrno = dump_csrs[i];
 target_ulong val = 0;
 RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0);
@@ -742,7 +742,7 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int 
flags)
 CSR_VTYPE,
 CSR_VLENB,
 };
-for (int i = 0; i < ARRAY_SIZE(dump_rvv_csrs); ++i) {
+for (i = 0; i < ARRAY_SIZE(dump_rvv_csrs); ++i) {
 int csrno = dump_rvv_csrs[i];
 target_ulong val = 0;
 RISCVException res = riscv_csrrw_debug(env, csrno, &val, 0, 0);
-- 
2.41.0

[PATCH 1/4] hw/riscv: opentitan: Fixup local variables shadowing

[Alistair Francis]

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

--extra-cflags='-Wshadow=local -Wno-error=shadow=local 
-Wno-error=shadow=compatible-local'

To configure

Signed-off-by: Alistair Francis 
---
 hw/riscv/opentitan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 6a2fcc4ade..436503f1ba 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -227,7 +227,7 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, 
Error **errp)
IRQ_M_TIMER));
 
 /* SPI-Hosts */
-for (int i = 0; i < OPENTITAN_NUM_SPI_HOSTS; ++i) {
+for (i = 0; i < OPENTITAN_NUM_SPI_HOSTS; ++i) {
 dev = DEVICE(&(s->spi_host[i]));
 if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi_host[i]), errp)) {
 return;
-- 
2.41.0

[PATCH 4/4] softmmu/device_tree: Fixup local variables shadowing

[Alistair Francis]

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

--extra-cflags='-Wshadow=local -Wno-error=shadow=local 
-Wno-error=shadow=compatible-local'

To configure

Signed-off-by: Alistair Francis 
---
 softmmu/device_tree.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index 30aa3aea9f..eb5166ca36 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -418,9 +418,9 @@ int qemu_fdt_setprop_string_array(void *fdt, const char 
*node_path,
 }
 p = str = g_malloc0(total_len);
 for (i = 0; i < len; i++) {
-int len = strlen(array[i]) + 1;
-pstrcpy(p, len, array[i]);
-p += len;
+int offset = strlen(array[i]) + 1;
+pstrcpy(p, offset, array[i]);
+p += offset;
 }
 
 ret = qemu_fdt_setprop(fdt, node_path, prop, str, total_len);
-- 
2.41.0

[PATCH 0/4] RISC-V: Work towards enabling -Wshadow=local

[Alistair Francis]

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

--extra-cflags='-Wshadow=local -Wno-error=shadow=local 
-Wno-error=shadow=compatible-local'

To configure

Alistair Francis (4):
  hw/riscv: opentitan: Fixup local variables shadowing
  target/riscv: cpu: Fixup local variables shadowing
  target/riscv: vector_helper: Fixup local variables shadowing
  softmmu/device_tree: Fixup local variables shadowing

 hw/riscv/opentitan.c | 2 +-
 softmmu/device_tree.c| 6 +++---
 target/riscv/cpu.c   | 4 ++--
 target/riscv/vector_helper.c | 7 ---
 4 files changed, 10 insertions(+), 9 deletions(-)

-- 
2.41.0

[PATCH 3/4] target/riscv: vector_helper: Fixup local variables shadowing

[Alistair Francis]

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

--extra-cflags='-Wshadow=local -Wno-error=shadow=local 
-Wno-error=shadow=compatible-local'

To configure

Signed-off-by: Alistair Francis 
---
 target/riscv/vector_helper.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 3fb05cc3d6..cba02c1320 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -516,7 +516,7 @@ ProbeSuccess:
 k++;
 continue;
 }
-target_ulong addr = base + ((i * nf + k) << log2_esz);
+addr = base + ((i * nf + k) << log2_esz);
 ldst_elem(env, adjust_addr(env, addr), i + k * max_elems, vd, ra);
 k++;
 }
@@ -4791,9 +4791,10 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, 
void *vs2, \
 uint32_t total_elems = vext_get_total_elems(env, desc, esz);  \
 uint32_t vta = vext_vta(desc);\
 uint32_t vma = vext_vma(desc);\
-target_ulong i_max, i;\
+target_ulong i_max, i_min, i; \
   \
-i_max = MAX(MIN(s1 < vlmax ? vlmax - s1 : 0, vl), env->vstart);   \
+i_min = MIN(s1 < vlmax ? vlmax - s1 : 0, vl); \
+i_max = MAX(i_min, env->vstart);  \
 for (i = env->vstart; i < i_max; ++i) {   \
 if (!vm && !vext_elem_mask(v0, i)) {  \
 /* set masked-off elements to 1s */   \
-- 
2.41.0

Re: [PATCH 22/52] migration/rdma: Fix rdma_getaddrinfo() error checking

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> rdma_getaddrinfo() returns 0 on success.  On error, it returns one of
> the EAI_ error codes like getaddrinfo() does,


Good catch, It used to be -1 on error, rdma_getaddrinfo(3) updated it 2021.



  or -1 with errno set.
> This is broken by design: POSIX implicitly specifies the EAI_ error
> codes to be non-zero, no more.  They could clash with -1.  Nothing we
> can do about this design flaw.
> 
> Both callers of rdma_getaddrinfo() only recognize negative values as
> error.  Works only because systems elect to make the EAI_ error codes
> negative.
> 
> Best not to rely on that: change the callers to treat any non-zero
> value as failure.  Also change them to return -1 instead of the value
> received from getaddrinfo() on failure, to avoid positive error
> values.
> 
> Signed-off-by: Markus Armbruster 


Reviewed-by: Li Zhijian 


> ---
>   migration/rdma.c | 14 ++
>   1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 46b5859268..3421ae0796 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -935,14 +935,14 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, 
> Error **errp)
>   
>   if (rdma->host == NULL || !strcmp(rdma->host, "")) {
>   ERROR(errp, "RDMA hostname has not been set");
> -return -EINVAL;
> +return -1;
>   }
>   
>   /* create CM channel */
>   rdma->channel = rdma_create_event_channel();
>   if (!rdma->channel) {
>   ERROR(errp, "could not create CM channel");
> -return -EINVAL;
> +return -1;
>   }
>   
>   /* create CM id */
> @@ -956,7 +956,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, 
> Error **errp)
>   port_str[15] = '\0';
>   
>   ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> -if (ret < 0) {
> +if (ret) {
>   ERROR(errp, "could not rdma_getaddrinfo address %s", rdma->host);
>   goto err_resolve_get_addr;
>   }
> @@ -998,7 +998,6 @@ route:
>   rdma_event_str(cm_event->event));
>   error_report("rdma_resolve_addr");
>   rdma_ack_cm_event(cm_event);
> -ret = -EINVAL;
>   goto err_resolve_get_addr;
>   }
>   rdma_ack_cm_event(cm_event);
> @@ -1019,7 +1018,6 @@ route:
>   ERROR(errp, "result not equal to event_route_resolved: %s",
>   rdma_event_str(cm_event->event));
>   rdma_ack_cm_event(cm_event);
> -ret = -EINVAL;
>   goto err_resolve_get_addr;
>   }
>   rdma_ack_cm_event(cm_event);
> @@ -1034,7 +1032,7 @@ err_resolve_get_addr:
>   err_resolve_create_id:
>   rdma_destroy_event_channel(rdma->channel);
>   rdma->channel = NULL;
> -return ret;
> +return -1;
>   }
>   
>   /*
> @@ -2644,7 +2642,7 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error 
> **errp)
>   port_str[15] = '\0';
>   
>   ret = rdma_getaddrinfo(rdma->host, port_str, NULL, &res);
> -if (ret < 0) {
> +if (ret) {
>   ERROR(errp, "could not rdma_getaddrinfo address %s", rdma->host);
>   goto err_dest_init_bind_addr;
>   }
> @@ -2688,7 +2686,7 @@ err_dest_init_create_listen_id:
>   rdma_destroy_event_channel(rdma->channel);
>   rdma->channel = NULL;
>   rdma->error_state = ret;
> -return ret;
> +return -1;
>   
>   }
>   

Re: [PATCH 23/52] migration/rdma: Clean up qemu_rdma_wait_comp_channel()'s error value

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> qemu_rdma_wait_comp_channel() returns 0 on success, and either -1 or
> rdma->error_state on failure.  Callers actually expect a negative
> error value. 

I don't see the only one callers expect a negative error code.
migration/rdma.c:1654:ret = qemu_rdma_wait_comp_channel(rdma, ch);
migration/rdma.c-1655-if (ret) {
migration/rdma.c-1656-goto err_block_for_wrid;


  I believe rdma->error_state can't be positive, but let's
> make things more obvious by simply returning -1 on any failure.
> 
> Signed-off-by: Markus Armbruster 
> ---
>   migration/rdma.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 3421ae0796..efbb3c7754 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1588,7 +1588,7 @@ static int qemu_rdma_wait_comp_channel(RDMAContext 
> *rdma,
>   if (rdma->received_error) {
>   return -EPIPE;
>   }
> -return rdma->error_state;
> +return -!!rdma->error_state;

And i rarely see such things, below would be more clear.

return rdma->error_state ? -1 : 0:


>   }
>   
>   static struct ibv_comp_channel *to_channel(RDMAContext *rdma, uint64_t wrid)

RE: [PATCH v2 2/4] vfio/pci: enable vector on dynamic MSI-X allocation

[Liu, Jing2]

Hi Alex,

> On Sat, 9/23/2023 4:55 AM, Alex Williamson  wrote:
> On Mon, 18 Sep 2023 05:45:05 -0400
> Jing Liu  wrote:
> 
> > The vector_use callback is used to enable vector that is unmasked in
> > guest. The kernel used to only support static MSI-X allocation. When
> > allocating a new interrupt using "static MSI-X allocation" kernels,
> > QEMU first disables all previously allocated vectors and then
> > re-allocates all including the new one. The nr_vectors of
> > VFIOPCIDevice indicates that all vectors from 0 to nr_vectors are
> > allocated (and may be enabled), which is used to to loop all the
> > possibly used vectors
>  ^^ ^^
> 
> s/to to/to/

Will change.
> 
> > When, e.g., disabling MSI-X interrupts.
> >
> > Extend the vector_use function to support dynamic MSI-X allocation
> > when host supports the capability. QEMU therefore can individually
> > allocate and enable a new interrupt without affecting others or
> > causing interrupts lost during runtime.
> >
> > Utilize nr_vectors to calculate the upper bound of enabled vectors in
> > dynamic MSI-X allocation mode since looping all msix_entries_nr is not
> > efficient and unnecessary.
> >
> > Signed-off-by: Jing Liu 
> > Tested-by: Reinette Chatre 
> > ---
> > Changes since v1:
> > - Revise Qemu to QEMU.
> >
> > Changes since RFC v1:
> > - Test vdev->msix->noresize to identify the allocation mode. (Alex)
> > - Move defer_kvm_irq_routing test out and update nr_vectors in a
> >   common place before vfio_enable_vectors(). (Alex)
> > - Revise the comments. (Alex)
> > ---
> >  hw/vfio/pci.c | 44 +++-
> >  1 file changed, 27 insertions(+), 17 deletions(-)
> >
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index
> > 60654ca28ab8..84987e46fd7a 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -470,6 +470,7 @@ static int vfio_msix_vector_do_use(PCIDevice *pdev,
> unsigned int nr,
> >  VFIOPCIDevice *vdev = VFIO_PCI(pdev);
> >  VFIOMSIVector *vector;
> >  int ret;
> > +int old_nr_vecs = vdev->nr_vectors;
> 
> Minor suggestion, it reads slightly better below if this were something
> like:
> 
> bool resizing = !!(vdev->nr_vectors < nr + 1);
> 
> Then use the bool in place of the nr+1 tests below.  Thanks,
> 
Got it. This change makes it nice to read. Thanks for the advice. Will send v3 
later. 

Thanks,
Jing

> Alex
> 
> >
> >  trace_vfio_msix_vector_do_use(vdev->vbasedev.name, nr);
> >
> > @@ -512,33 +513,42 @@ static int vfio_msix_vector_do_use(PCIDevice
> *pdev, unsigned int nr,
> >  }
> >
> >  /*
> > - * We don't want to have the host allocate all possible MSI vectors
> > - * for a device if they're not in use, so we shutdown and incrementally
> > - * increase them as needed.
> > + * When dynamic allocation is not supported, we don't want to have the
> > + * host allocate all possible MSI vectors for a device if they're not
> > + * in use, so we shutdown and incrementally increase them as needed.
> > + * nr_vectors represents the total number of vectors allocated.
> > + *
> > + * When dynamic allocation is supported, let the host only allocate
> > + * and enable a vector when it is in use in guest. nr_vectors 
> > represents
> > + * the upper bound of vectors being enabled (but not all of the ranges
> > + * is allocated or enabled).
> >   */
> >  if (vdev->nr_vectors < nr + 1) {
> >  vdev->nr_vectors = nr + 1;
> > -if (!vdev->defer_kvm_irq_routing) {
> > +}
> > +
> > +if (!vdev->defer_kvm_irq_routing) {
> > +if (vdev->msix->noresize && (old_nr_vecs < nr + 1)) {
> >  vfio_disable_irqindex(&vdev->vbasedev, 
> > VFIO_PCI_MSIX_IRQ_INDEX);
> >  ret = vfio_enable_vectors(vdev, true);
> >  if (ret) {
> >  error_report("vfio: failed to enable vectors, %d", ret);
> >  }
> > -}
> > -} else {
> > -Error *err = NULL;
> > -int32_t fd;
> > -
> > -if (vector->virq >= 0) {
> > -fd = event_notifier_get_fd(&vector->kvm_interrupt);
> >  } else {
> > -fd = event_notifier_get_fd(&vector->interrupt);
> > -}
> > +Error *err = NULL;
> > +int32_t fd;
> >
> > -if (vfio_set_irq_signaling(&vdev->vbasedev,
> > - VFIO_PCI_MSIX_IRQ_INDEX, nr,
> > - VFIO_IRQ_SET_ACTION_TRIGGER, fd, 
> > &err)) {
> > -error_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
> > +if (vector->virq >= 0) {
> > +fd = event_notifier_get_fd(&vector->kvm_interrupt);
> > +} else {
> > +fd = event_notifier_get_fd(&vector->interrupt);
> > +}
> > +
> > +if (vfio_set_irq_signaling(&vdev->vbasedev,
> > +   VFIO_PCI_MSIX_IRQ_INDEX, nr,
> > +  

[PATCH] hw/nvme: Clean up local variable shadowing in nvme_ns_init()

[Klaus Jensen]

From: Klaus Jensen 

Fix local variable shadowing in nvme_ns_init().

Reported-by: Markus Armbruster 
Signed-off-by: Klaus Jensen 
---
 hw/nvme/ns.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/nvme/ns.c b/hw/nvme/ns.c
index 44aba8f4d9cf..0eabcf5cf500 100644
--- a/hw/nvme/ns.c
+++ b/hw/nvme/ns.c
@@ -107,7 +107,7 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
 
 ns->pif = ns->params.pif;
 
-static const NvmeLBAF lbaf[16] = {
+static const NvmeLBAF defaults[16] = {
 [0] = { .ds =  9   },
 [1] = { .ds =  9, .ms =  8 },
 [2] = { .ds =  9, .ms = 16 },
@@ -120,7 +120,7 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
 
 ns->nlbaf = 8;
 
-memcpy(&id_ns->lbaf, &lbaf, sizeof(lbaf));
+memcpy(&id_ns->lbaf, &defaults, sizeof(defaults));
 
 for (i = 0; i < ns->nlbaf; i++) {
 NvmeLBAF *lbaf = &id_ns->lbaf[i];

---
base-commit: b55e4b9c0525560577384adfc6d30eb0daa8d7be
change-id: 20230925-fix-local-shadowing-9606793e8ae9

Best regards,
-- 
Klaus Jensen 

RE: [PATCH v2 0/4] Support dynamic MSI-X allocation

[Liu, Jing2]

Hi Alex,

> On Sat, 9/23/2023 4:57AM, Alex Williamson  wrote:
> 
> On Mon, 18 Sep 2023 05:45:03 -0400
> Jing Liu  wrote:
> 
> > Changes since v1:
> > - v1:
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg982842.html
> > - Revise Qemu to QEMU. (Cédric)
> > - Add g_free when failure of getting MSI-X irq info. (Cédric)
> > - Apply Cédric's Reviewed-by. (Cédric)
> > - Use g_autofree to automatically release. (Cédric)
> > - Remove the failure message in vfio_enable_msix_no_vec(). (Cédric)
> >
> > Changes since RFC v1:
> > - RFC v1:
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg978637.html
> > - Revise the comments. (Alex)
> > - Report error of getting irq info and remove the trace of failure
> >   case. (Alex, Cédric)
> > - Only store dynamic allocation flag as a bool type and test
> >   accordingly. (Alex)
> > - Move dynamic allocation detection to vfio_msix_early_setup(). (Alex)
> > - Change the condition logic in vfio_msix_vector_do_use() that moving
> >   the defer_kvm_irq_routing test out and create a common place to update
> >   nr_vectors. (Alex)
> > - Consolidate the way of MSI-X enabling during device initialization and
> >   interrupt restoring that uses fd = -1 trick. Create a function doing
> >   that. (Alex)
> >
> > Before kernel v6.5, dynamic allocation of MSI-X interrupts was not
> > supported. QEMU therefore when allocating a new interrupt, should
> > first release all previously allocated interrupts (including disable
> > of MSI-X) and re-allocate all interrupts that includes the new one.
> >
> > The kernel series [1] adds the support of dynamic MSI-X allocation to
> > vfio-pci and uses the existing flag VFIO_IRQ_INFO_NORESIZE to guide
> > user space, that when dynamic MSI-X is supported the flag is cleared.
> >
> > This series makes the behavior for VFIO PCI devices when dynamic MSI-X
> > allocation is supported. When guest unmasks an interrupt, QEMU can
> > directly allocate an interrupt on host for this and has nothing to do
> > with the previously allocated ones. Therefore, host only allocates
> > interrupts for those unmasked (enabled) interrupts inside guest when
> > dynamic MSI-X allocation is supported by device.
> >
> > When guests enable MSI-X with all of the vectors masked, QEMU need
> > match the state to enable MSI-X with no vector enabled. During
> > migration restore, QEMU also need enable MSI-X first in dynamic
> > allocation mode, to avoid the guest unused vectors being allocated on
> > host. To consolidate them, we use vector 0 with an invalid fd to get
> > MSI-X enabled and create a common function for this. This is cleaner
> > than setting userspace triggering and immediately release.
> >
> > Any feedback is appreciated.
> >
> > Jing
> >
> > [1] https://lwn.net/Articles/931679/
> >
> > Jing Liu (4):
> >   vfio/pci: detect the support of dynamic MSI-X allocation
> >   vfio/pci: enable vector on dynamic MSI-X allocation
> >   vfio/pci: use an invalid fd to enable MSI-X
> >   vfio/pci: enable MSI-X in interrupt restoring on dynamic allocation
> >
> >  hw/vfio/pci.c| 121 +--
> >  hw/vfio/pci.h|   1 +
> >  hw/vfio/trace-events |   2 +-
> >  3 files changed, 96 insertions(+), 28 deletions(-)
> >
> 
> Some minor comments on 2/ but otherwise:
> 
> Reviewed-by: Alex Williamson 

Thank you very much for the feedback. Will apply on v3 with fix for 2/4. 

Jing

Re: [PATCH 26/52] migration/rdma: Replace int error_state by bool errored

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> All we do with the value of RDMAContext member @error_state is test
> whether it's zero.  Change to bool and rename to @errored.
> 

make sense!

Reviewed-by: Li Zhijian 

Can we move this patch ahead "[PATCH 23/52] migration/rdma: Clean up 
qemu_rdma_wait_comp_channel()'s error value",
so that [23/52] [24/52] [25/52] will be more easy to review.



> Signed-off-by: Markus Armbruster 
> ---
>   migration/rdma.c | 66 
>   1 file changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index ad314cc10a..85f6b274bf 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -352,7 +352,7 @@ typedef struct RDMAContext {
>* memory registration, then do not attempt any future work
>* and remember the error state.
>*/
> -int error_state;
> +int errored;
>   bool error_reported;
>   bool received_error;
>   
> @@ -439,14 +439,14 @@ typedef struct QEMU_PACKED {
>   uint64_t chunks;/* how many sequential chunks to register */
>   } RDMARegister;
>   
> -static int check_error_state(RDMAContext *rdma)
> +static bool rdma_errored(RDMAContext *rdma)
>   {
> -if (rdma->error_state && !rdma->error_reported) {
> +if (rdma->errored && !rdma->error_reported) {
>   error_report("RDMA is in an error state waiting migration"
>" to abort!");
>   rdma->error_reported = true;
>   }
> -return rdma->error_state;
> +return rdma->errored;
>   }
>   
>   static void register_to_network(RDMAContext *rdma, RDMARegister *reg)
> @@ -1531,7 +1531,7 @@ static int qemu_rdma_wait_comp_channel(RDMAContext 
> *rdma,
>* But we need to be able to handle 'cancel' or an error
>* without hanging forever.
>*/
> -while (!rdma->error_state  && !rdma->received_error) {
> +while (!rdma->errored && !rdma->received_error) {
>   GPollFD pfds[2];
>   pfds[0].fd = comp_channel->fd;
>   pfds[0].events = G_IO_IN | G_IO_HUP | G_IO_ERR;
> @@ -1588,7 +1588,7 @@ static int qemu_rdma_wait_comp_channel(RDMAContext 
> *rdma,
>   if (rdma->received_error) {
>   return -1;
>   }
> -return -!!rdma->error_state;
> +return -rdma->errored;
>   }
>   
>   static struct ibv_comp_channel *to_channel(RDMAContext *rdma, uint64_t wrid)
> @@ -1701,7 +1701,7 @@ err_block_for_wrid:
>   ibv_ack_cq_events(cq, num_cq_events);
>   }
>   
> -rdma->error_state = ret;
> +rdma->errored = true;
>   return -1;
>   }
>   
> @@ -2340,7 +2340,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
>   int idx;
>   
>   if (rdma->cm_id && rdma->connected) {
> -if ((rdma->error_state ||
> +if ((rdma->errored ||
>migrate_get_current()->state == MIGRATION_STATUS_CANCELLING) &&
>   !rdma->received_error) {
>   RDMAControlHeader head = { .len = 0,
> @@ -2621,14 +2621,14 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, 
> Error **errp)
>   
>   if (!rdma->host || !rdma->host[0]) {
>   ERROR(errp, "RDMA host is not set!");
> -rdma->error_state = -EINVAL;
> +rdma->errored = true;
>   return -1;
>   }
>   /* create CM channel */
>   rdma->channel = rdma_create_event_channel();
>   if (!rdma->channel) {
>   ERROR(errp, "could not create rdma event channel");
> -rdma->error_state = -EINVAL;
> +rdma->errored = true;
>   return -1;
>   }
>   
> @@ -2686,7 +2686,7 @@ err_dest_init_bind_addr:
>   err_dest_init_create_listen_id:
>   rdma_destroy_event_channel(rdma->channel);
>   rdma->channel = NULL;
> -rdma->error_state = ret;
> +rdma->errored = true;
>   return -1;
>   
>   }
> @@ -2763,7 +2763,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
>   return -1;
>   }
>   
> -if (rdma->error_state) {
> +if (rdma->errored) {
>   error_setg(errp,
>  "RDMA is in an error state waiting migration to abort!");
>   return -1;
> @@ -2775,7 +2775,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
>*/
>   ret = qemu_rdma_write_flush(f, rdma);
>   if (ret < 0) {
> -rdma->error_state = ret;
> +rdma->errored = true;
>   error_setg(errp, "qemu_rdma_write_flush failed");
>   return -1;
>   }
> @@ -2795,7 +2795,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
>   ret = qemu_rdma_exchange_send(rdma, &head, data, NULL, NULL, 
> NULL);
>   
>   if (ret < 0) {
> -rdma->error_state = ret;
> +rdma->errored = true;
>   error_setg(errp, "qemu_rdma_exchange_send failed");
>   return -1;
>   }
> @@ -2853,7 +2853,7 @@ static ssize_t qio_channel_rd

Re: [PATCH 27/52] migration/rdma: Drop superfluous assignments to @ret

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster 

Reviewed-by: Li Zhijian 


> ---
>   migration/rdma.c | 35 ++-
>   1 file changed, 10 insertions(+), 25 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 85f6b274bf..62d95b7d2c 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1514,7 +1514,7 @@ static int qemu_rdma_wait_comp_channel(RDMAContext 
> *rdma,
>  struct ibv_comp_channel 
> *comp_channel)
>   {
>   struct rdma_cm_event *cm_event;
> -int ret = -1;
> +int ret;
>   
>   /*
>* Coroutine doesn't start until migration_fd_process_incoming()
> @@ -1619,7 +1619,7 @@ static int qemu_rdma_block_for_wrid(RDMAContext *rdma,
>   uint64_t wrid_requested,
>   uint32_t *byte_len)
>   {
> -int num_cq_events = 0, ret = 0;
> +int num_cq_events = 0, ret;
>   struct ibv_cq *cq;
>   void *cq_ctx;
>   uint64_t wr_id = RDMA_WRID_NONE, wr_id_in;
> @@ -1664,8 +1664,7 @@ static int qemu_rdma_block_for_wrid(RDMAContext *rdma,
>   
>   num_cq_events++;
>   
> -ret = -ibv_req_notify_cq(cq, 0);
> -if (ret) {
> +if (ibv_req_notify_cq(cq, 0)) {
>   goto err_block_for_wrid;
>   }
>   
> @@ -1712,7 +1711,7 @@ err_block_for_wrid:
>   static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf,
>  RDMAControlHeader *head)
>   {
> -int ret = 0;
> +int ret;
>   RDMAWorkRequestData *wr = &rdma->wr_data[RDMA_WRID_CONTROL];
>   struct ibv_send_wr *bad_wr;
>   struct ibv_sge sge = {
> @@ -1869,7 +1868,7 @@ static int qemu_rdma_exchange_send(RDMAContext *rdma, 
> RDMAControlHeader *head,
>  int *resp_idx,
>  int (*callback)(RDMAContext *rdma))
>   {
> -int ret = 0;
> +int ret;
>   
>   /*
>* Wait until the dest is ready before attempting to deliver the message
> @@ -2841,7 +2840,7 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc,
>   QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
>   RDMAContext *rdma;
>   RDMAControlHeader head;
> -int ret = 0;
> +int ret;
>   size_t i;
>   size_t done = 0;
>   
> @@ -3340,7 +3339,7 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>   RDMAContext *rdma_return_path = NULL;
>   struct rdma_cm_event *cm_event;
>   struct ibv_context *verbs;
> -int ret = -EINVAL;
> +int ret;
>   int idx;
>   
>   ret = rdma_get_cm_event(rdma->channel, &cm_event);
> @@ -3350,7 +3349,6 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>   
>   if (cm_event->event != RDMA_CM_EVENT_CONNECT_REQUEST) {
>   rdma_ack_cm_event(cm_event);
> -ret = -1;
>   goto err_rdma_dest_wait;
>   }
>   
> @@ -3363,7 +3361,6 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>   rdma_return_path = qemu_rdma_data_init(rdma->host_port, NULL);
>   if (rdma_return_path == NULL) {
>   rdma_ack_cm_event(cm_event);
> -ret = -1;
>   goto err_rdma_dest_wait;
>   }
>   
> @@ -3378,7 +3375,6 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>   error_report("Unknown source RDMA version: %d, bailing...",
>cap.version);
>   rdma_ack_cm_event(cm_event);
> -ret = -1;
>   goto err_rdma_dest_wait;
>   }
>   
> @@ -3411,7 +3407,6 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>   } else if (rdma->verbs != verbs) {
>   error_report("ibv context not matching %p, %p!", rdma->verbs,
>verbs);
> -ret = -1;
>   goto err_rdma_dest_wait;
>   }
>   
> @@ -3465,7 +3460,6 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>   if (cm_event->event != RDMA_CM_EVENT_ESTABLISHED) {
>   error_report("rdma_accept not event established");
>   rdma_ack_cm_event(cm_event);
> -ret = -1;
>   goto err_rdma_dest_wait;
>   }
>   
> @@ -3528,7 +3522,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
>   static RDMARegisterResult 
> results[RDMA_CONTROL_MAX_COMMANDS_PER_MESSAGE];
>   RDMALocalBlock *block;
>   void *host_addr;
> -int ret = 0;
> +int ret;
>   int idx = 0;
>   int count = 0;
>   int i = 0;
> @@ -3557,7 +3551,6 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
>   if (head.repeat > RDMA_CONTROL_MAX_COMMANDS_PER_MESSAGE) {
>   error_report("rdma: Too many requests in this message (%d)."
>   "Bailing.", head.repeat);
> -ret = -EIO;
>   break;
>   }
>   
> @@ -3573,7 +3566,6 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
>   error_report("rdma: 'compress' bad blo

[PATCH] MAINTAINERS: aspeed: Update Andrew's email address

[Andrew Jeffery]

I've changed employers, have company email that deals with patch-based
workflows without too much of a headache, and am trying to steer some
content out of my personal mail.

Signed-off-by: Andrew Jeffery 
---

Hi Cédric, do you mind including this in your Aspeed queue?

 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 355b1960ce46..b142c09628b9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1108,7 +1108,7 @@ F: docs/system/arm/emcraft-sf2.rst
 ASPEED BMCs
 M: Cédric Le Goater 
 M: Peter Maydell 
-R: Andrew Jeffery 
+R: Andrew Jeffery 
 R: Joel Stanley 
 L: qemu-...@nongnu.org
 S: Maintained
-- 
2.39.2

Re: [PATCH 28/52] migration/rdma: Check negative error values the same way everywhere

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> When a function returns 0 on success, negative value on error,
> checking for non-zero suffices, but checking for negative is clearer.
> So do that.
> 

This patch is no my favor convention.

@Peter, Juan

I'd like to hear your opinions.

Thanks
Zhijian


> Signed-off-by: Markus Armbruster 
> ---
>   migration/rdma.c | 82 
>   1 file changed, 41 insertions(+), 41 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 62d95b7d2c..6c643a1b30 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -947,7 +947,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, 
> Error **errp)
>   
>   /* create CM id */
>   ret = rdma_create_id(rdma->channel, &rdma->cm_id, NULL, RDMA_PS_TCP);
> -if (ret) {
> +if (ret < 0) {
>   ERROR(errp, "could not create channel id");
>   goto err_resolve_create_id;
>   }
> @@ -968,10 +968,10 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, 
> Error **errp)
>   
>   ret = rdma_resolve_addr(rdma->cm_id, NULL, e->ai_dst_addr,
>   RDMA_RESOLVE_TIMEOUT_MS);
> -if (!ret) {
> +if (ret >= 0) {
>   if (e->ai_family == AF_INET6) {
>   ret = qemu_rdma_broken_ipv6_kernel(rdma->cm_id->verbs, 
> errp);
> -if (ret) {
> +if (ret < 0) {
>   continue;
>   }
>   }
> @@ -988,7 +988,7 @@ route:
>   qemu_rdma_dump_gid("source_resolve_addr", rdma->cm_id);
>   
>   ret = rdma_get_cm_event(rdma->channel, &cm_event);
> -if (ret) {
> +if (ret < 0) {
>   ERROR(errp, "could not perform event_addr_resolved");
>   goto err_resolve_get_addr;
>   }
> @@ -1004,13 +1004,13 @@ route:
>   
>   /* resolve route */
>   ret = rdma_resolve_route(rdma->cm_id, RDMA_RESOLVE_TIMEOUT_MS);
> -if (ret) {
> +if (ret < 0) {
>   ERROR(errp, "could not resolve rdma route");
>   goto err_resolve_get_addr;
>   }
>   
>   ret = rdma_get_cm_event(rdma->channel, &cm_event);
> -if (ret) {
> +if (ret < 0) {
>   ERROR(errp, "could not perform event_route_resolved");
>   goto err_resolve_get_addr;
>   }
> @@ -1118,7 +1118,7 @@ static int qemu_rdma_alloc_qp(RDMAContext *rdma)
>   attr.qp_type = IBV_QPT_RC;
>   
>   ret = rdma_create_qp(rdma->cm_id, rdma->pd, &attr);
> -if (ret) {
> +if (ret < 0) {
>   return -1;
>   }
>   
> @@ -1551,7 +1551,7 @@ static int qemu_rdma_wait_comp_channel(RDMAContext 
> *rdma,
>   
>   if (pfds[1].revents) {
>   ret = rdma_get_cm_event(rdma->channel, &cm_event);
> -if (ret) {
> +if (ret < 0) {
>   error_report("failed to get cm event while wait "
>"completion channel");
>   return -1;
> @@ -1652,12 +1652,12 @@ static int qemu_rdma_block_for_wrid(RDMAContext *rdma,
>   
>   while (1) {
>   ret = qemu_rdma_wait_comp_channel(rdma, ch);
> -if (ret) {
> +if (ret < 0) {
>   goto err_block_for_wrid;
>   }
>   
>   ret = ibv_get_cq_event(ch, &cq, &cq_ctx);
> -if (ret) {
> +if (ret < 0) {
>   perror("ibv_get_cq_event");
>   goto err_block_for_wrid;
>   }
> @@ -1888,7 +1888,7 @@ static int qemu_rdma_exchange_send(RDMAContext *rdma, 
> RDMAControlHeader *head,
>*/
>   if (resp) {
>   ret = qemu_rdma_post_recv_control(rdma, RDMA_WRID_DATA);
> -if (ret) {
> +if (ret < 0) {
>   error_report("rdma migration: error posting"
>   " extra control recv for anticipated result!");
>   return -1;
> @@ -1899,7 +1899,7 @@ static int qemu_rdma_exchange_send(RDMAContext *rdma, 
> RDMAControlHeader *head,
>* Post a WR to replace the one we just consumed for the READY message.
>*/
>   ret = qemu_rdma_post_recv_control(rdma, RDMA_WRID_READY);
> -if (ret) {
> +if (ret < 0) {
>   error_report("rdma migration: error posting first control recv!");
>   return -1;
>   }
> @@ -1986,7 +1986,7 @@ static int qemu_rdma_exchange_recv(RDMAContext *rdma, 
> RDMAControlHeader *head,
>* Post a new RECV work request to replace the one we just consumed.
>*/
>   ret = qemu_rdma_post_recv_control(rdma, RDMA_WRID_READY);
> -if (ret) {
> +if (ret < 0) {
>   error_report("rdma migration: error posting second control recv!");
>   return -1;
>   }
> @@ -2311,7 +2311,7 @@ static int qemu_rdma_write(QEMUFile *f, RDMAContext 
> *rdma,
>   /* If we cannot merge it, we flush the current buffer first. */
>   if (!qemu_rdma_buffer_mergable(rdma, current_addr, len)) {
>   ret = qemu_rd

Re: [PATCH 3/4] aspeed/i3c: Rename variable shadowing a local

[Andrew Jeffery]

On Sat, 23 Sep 2023, at 03:58, Philippe Mathieu-Daudé wrote:
> On 22/9/23 17:59, Cédric Le Goater wrote:
>> to fix warning :
>> 
>>../hw/i3c/aspeed_i3c.c: In function ‘aspeed_i3c_realize’:
>>../hw/i3c/aspeed_i3c.c:1959:17: warning: declaration of ‘dev’ shadows a 
>> parameter [-Wshadow=local]
>> 1959 | Object *dev = OBJECT(&s->devices[i]);
>>  | ^~~
>>../hw/i3c/aspeed_i3c.c:1942:45: note: shadowed declaration is here
>> 1942 | static void aspeed_i3c_realize(DeviceState *dev, Error **errp)
>>  |~^~~
>> 
>> Signed-off-by: Cédric Le Goater 
>> ---
>>   hw/misc/aspeed_i3c.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/hw/misc/aspeed_i3c.c b/hw/misc/aspeed_i3c.c
>> index f54f5da522b3..d1ff61767167 100644
>> --- a/hw/misc/aspeed_i3c.c
>> +++ b/hw/misc/aspeed_i3c.c
>> @@ -296,13 +296,13 @@ static void aspeed_i3c_realize(DeviceState *dev, Error 
>> **errp)
>
> Alternatively:
>
> -- >8 --
>
> -static void aspeed_i3c_realize(DeviceState *dev, Error **errp)
> +static void aspeed_i3c_realize(DeviceState *ctrl, Error **errp)
>   {
>   int i;
> -AspeedI3CState *s = ASPEED_I3C(dev);
> -SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +AspeedI3CState *s = ASPEED_I3C(ctrl);
> +SysBusDevice *sbd = SYS_BUS_DEVICE(ctrl);

Hmm, I feel like `dev` isn't an unreasonable or unusual name for the formal 
parameter? Switching to `ctrl` isn't my immediate preference.

>
> ---
>
>>   memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);
>>   
>>   for (i = 0; i < ASPEED_I3C_NR_DEVICES; ++i) {
>> -Object *dev = OBJECT(&s->devices[i]);
>> +Object *i3c_dev = OBJECT(&s->devices[i]);

Maybe `s/i3c_dev/subdev`? I dunno. That bikeshed isn't gonna get painted on its 
own.

Reviewed-by: Andrew Jeffery 

>>   
>> -if (!object_property_set_uint(dev, "device-id", i, errp)) {
>> +if (!object_property_set_uint(i3c_dev, "device-id", i, errp)) {
>>   return;
>>   }
>>   
>> -if (!sysbus_realize(SYS_BUS_DEVICE(dev), errp)) {
>> +if (!sysbus_realize(SYS_BUS_DEVICE(i3c_dev), errp)) {
>>   return;
>>   }
>>   
>
> Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 29/52] migration/rdma: Plug a memory leak and improve a message

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> When migration capability @rdma-pin-all is true, but the server cannot
> honor it, qemu_rdma_connect() calls macro ERROR(), then returns
> success.
> 
> ERROR() sets an error.  Since qemu_rdma_connect() returns success, its
> caller rdma_start_outgoing_migration() duly assumes @errp is still
> clear.  The Error object leaks.
> 
> ERROR() additionally reports the situation to the user as an error:
> 
>  RDMA ERROR: Server cannot support pinning all memory. Will register 
> memory dynamically.
> 
> Is this an error or not?  It actually isn't; we disable @rdma-pin-all
> and carry on.  "Correcting" the user's configuration decisions that
> way feels problematic, but that's a topic for another day.
> 
> Replace ERROR() by warn_report().  This plugs the memory leak, and
> emits a clearer message to the user.
> 
> Signed-off-by: Markus Armbruster 


Reviewed-by: Li Zhijian 

> ---
>   migration/rdma.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 6c643a1b30..d52de857c5 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2586,8 +2586,8 @@ static int qemu_rdma_connect(RDMAContext *rdma, bool 
> return_path,
>* and disable them otherwise.
>*/
>   if (rdma->pin_all && !(cap.flags & RDMA_CAPABILITY_PIN_ALL)) {
> -ERROR(errp, "Server cannot support pinning all memory. "
> -"Will register memory dynamically.");
> +warn_report("RDMA: Server cannot support pinning all memory. "
> +"Will register memory dynamically.");
>   rdma->pin_all = false;
>   }
>   

Re: [PATCH 30/52] migration/rdma: Delete inappropriate error_report() in macro ERROR()

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> Functions that use an Error **errp parameter to return errors should
> not also report them to the user, because reporting is the caller's
> job.  When the caller does, the error is reported twice.  When it
> doesn't (because it recovered from the error), there is no error to
> report, i.e. the report is bogus.
> 
> Macro ERROR() violates this principle.  Delete the error_report()
> there.
> 
> Signed-off-by: Markus Armbruster 

Reviewed-by: Li Zhijian 

And

Tested-by: Li Zhijian 

> ---
>   migration/rdma.c | 4 
>   1 file changed, 4 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index d52de857c5..be31694d4f 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -40,12 +40,8 @@
>   #include "options.h"
>   #include 
>   
> -/*
> - * Print and error on both the Monitor and the Log file.
> - */
>   #define ERROR(errp, fmt, ...) \
>   do { \
> -fprintf(stderr, "RDMA ERROR: " fmt "\n", ## __VA_ARGS__); \
>   if (errp && (*(errp) == NULL)) { \
>   error_setg(errp, "RDMA ERROR: " fmt, ## __VA_ARGS__); \
>   } \

Re: [PATCH 21/52] migration/rdma: Fix QEMUFileHooks method return values

[Markus Armbruster]

"Zhijian Li (Fujitsu)"  writes:

> On 18/09/2023 22:41, Markus Armbruster wrote:
>> The QEMUFileHooks methods don't come with a written contract.  Digging
>> through the code calling them, we find:
>> 
>> * save_page():
>
> I'm fine with this
>
>> 
>>Negative values RAM_SAVE_CONTROL_DELAYED and
>>RAM_SAVE_CONTROL_NOT_SUPP are special.  Any other negative value is
>>an unspecified error.
>> 
>>qemu_rdma_save_page() returns -EIO or rdma->error_state on error.  I
>>believe the latter is always negative.  Nothing stops either of them
>>to clash with the special values, though.  Feels unlikely, but fix
>>it anyway to return only the special values and -1.
>> 
>> * before_ram_iterate(), before_ram_iterate():
>
> error code returned by before_ram_iterate() and before_ram_iterate() will be
> assigned to QEMUFile for upper layer.
> But it seems that no callers take care about the error ?  Shouldn't let the 
> callers
> to check the error instead ?

The error values returned by qemu_rdma_registration_start() and
qemu_rdma_registration_stop() carry no additional information a caller
could check.

Both return either -EIO or rdma->error_state on error.  The latter is
*not* a negative errno code.  Evidence: qio_channel_rdma_shutdown()
assigns -1, qemu_rdma_block_for_wrid() assigns the error value of
qemu_rdma_poll(), which can be the error value of ibv_poll_cq(), which
is an unspecified negative value, ...

I decided not to investigate what qemu-file.c does with the error values
after one quick glance at the code.  It's confusing, and quite possibly
confused.  But I'm already at 50+ patches, and am neither inclined nor
able to take on more migration cleanup at this time.

>>Negative value means error.  qemu_rdma_registration_start() and
>>qemu_rdma_registration_stop() comply as far as I can tell.  Make
>>them comply *obviously*, by returning -1 on error.
>> 
>> * hook_ram_load:
>> 
>>Negative value means error.  rdma_load_hook() already returns -1 on
>>error.  Leave it alone.
>
> see inline reply below,
>
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>   migration/rdma.c | 79 +++-
>>   1 file changed, 37 insertions(+), 42 deletions(-)
>> 
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index cc59155a50..46b5859268 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -3219,12 +3219,11 @@ static size_t qemu_rdma_save_page(QEMUFile *f,
>>   rdma = qatomic_rcu_read(&rioc->rdmaout);
>>   
>>   if (!rdma) {
>> -return -EIO;
>> +return -1;
>>   }
>>   
>> -ret = check_error_state(rdma);
>> -if (ret) {
>> -return ret;
>> +if (check_error_state(rdma)) {
>> +return -1;
>>   }
>>   
>>   qemu_fflush(f);
>> @@ -3290,9 +3289,10 @@ static size_t qemu_rdma_save_page(QEMUFile *f,
>>   }
>>   
>>   return RAM_SAVE_CONTROL_DELAYED;
>> +
>>   err:
>>   rdma->error_state = ret;
>> -return ret;
>> +return -1;
>>   }
>>   
>>   static void rdma_accept_incoming_migration(void *opaque);
>> @@ -3538,12 +3538,11 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
>>   rdma = qatomic_rcu_read(&rioc->rdmain);
>>   
>>   if (!rdma) {
>> -return -EIO;
>> +return -1;
>
> that's because EIO is not accurate here ?

It's because the function does not return a negative errno code, and
returning -EIO is misleading readers into assuming it does.

>>   }
>>   
>> -ret = check_error_state(rdma);
>> -if (ret) {
>> -return ret;
>
> Ditto

Likewise.

[...]

Re: [PATCH 23/52] migration/rdma: Clean up qemu_rdma_wait_comp_channel()'s error value

[Markus Armbruster]

"Zhijian Li (Fujitsu)"  writes:

> On 18/09/2023 22:41, Markus Armbruster wrote:
>> qemu_rdma_wait_comp_channel() returns 0 on success, and either -1 or
>> rdma->error_state on failure.  Callers actually expect a negative
>> error value. 
>
> I don't see the only one callers expect a negative error code.
> migration/rdma.c:1654:ret = qemu_rdma_wait_comp_channel(rdma, ch);
> migration/rdma.c-1655-if (ret) {
> migration/rdma.c-1656-goto err_block_for_wrid;

You're right.

I want the change anyway, to let me simplify the code some.  I'll adjust
the commit message.

>   I believe rdma->error_state can't be positive, but let's
>> make things more obvious by simply returning -1 on any failure.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>   migration/rdma.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index 3421ae0796..efbb3c7754 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -1588,7 +1588,7 @@ static int qemu_rdma_wait_comp_channel(RDMAContext 
>> *rdma,
>>   if (rdma->received_error) {
>>   return -EPIPE;
>>   }
>> -return rdma->error_state;
>> +return -!!rdma->error_state;
>
> And i rarely see such things, below would be more clear.
>
> return rdma->error_state ? -1 : 0:

Goes away in PATCH 26:

   @@ -1588,7 +1588,7 @@ static int qemu_rdma_wait_comp_channel(RDMAContext 
*rdma,
if (rdma->received_error) {
return -1;
}
   -return -!!rdma->error_state;
   +return -rdma->errored;
}

static struct ibv_comp_channel *to_channel(RDMAContext *rdma, uint64_t wrid)

>
>>   }
>>   
>>   static struct ibv_comp_channel *to_channel(RDMAContext *rdma, uint64_t 
>> wrid)