All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] gpio: of: Extract of_gpiochip_add_hog()
From: Geert Uytterhoeven @ 2020-02-20 13:01 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Pantelis Antoniou,
	Frank Rowand, Rob Herring, Mark Rutland
  Cc: Peter Ujfalusi, Chris Brandt, linux-gpio, devicetree,
	linux-renesas-soc, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20200220130149.26283-1-geert+renesas@glider.be>

Extract the code to add all GPIO hogs of a gpio-hog node into its own
function, so it can be reused.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/gpio/gpiolib-of.c | 49 ++++++++++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index c6d30f73df078e0b..2b47f93886075294 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -604,6 +604,35 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
 	return desc;
 }
 
+/**
+ * of_gpiochip_add_hog - Add all hogs in a hog device node
+ * @chip:	gpio chip to act on
+ * @hog:	device node describing the hogs
+ *
+ * Returns error if it fails otherwise 0 on success.
+ */
+static int of_gpiochip_add_hog(struct gpio_chip *chip, struct device_node *hog)
+{
+	enum gpiod_flags dflags;
+	struct gpio_desc *desc;
+	unsigned long lflags;
+	const char *name;
+	unsigned int i;
+	int ret;
+
+	for (i = 0;; i++) {
+		desc = of_parse_own_gpio(hog, chip, i, &name, &lflags, &dflags);
+		if (IS_ERR(desc))
+			break;
+
+		ret = gpiod_hog(desc, name, lflags, dflags);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 /**
  * of_gpiochip_scan_gpios - Scan gpio-controller for gpio definitions
  * @chip:	gpio chip to act on
@@ -614,29 +643,17 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
  */
 static int of_gpiochip_scan_gpios(struct gpio_chip *chip)
 {
-	struct gpio_desc *desc = NULL;
 	struct device_node *np;
-	const char *name;
-	unsigned long lflags;
-	enum gpiod_flags dflags;
-	unsigned int i;
 	int ret;
 
 	for_each_available_child_of_node(chip->of_node, np) {
 		if (!of_property_read_bool(np, "gpio-hog"))
 			continue;
 
-		for (i = 0;; i++) {
-			desc = of_parse_own_gpio(np, chip, i, &name, &lflags,
-						 &dflags);
-			if (IS_ERR(desc))
-				break;
-
-			ret = gpiod_hog(desc, name, lflags, dflags);
-			if (ret < 0) {
-				of_node_put(np);
-				return ret;
-			}
+		ret = of_gpiochip_add_hog(chip, np);
+		if (ret < 0) {
+			of_node_put(np);
+			return ret;
 		}
 	}
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 2/2] gpio: of: Add DT overlay support for GPIO hogs
From: Geert Uytterhoeven @ 2020-02-20 13:01 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Pantelis Antoniou,
	Frank Rowand, Rob Herring, Mark Rutland
  Cc: Peter Ujfalusi, Chris Brandt, linux-gpio, devicetree,
	linux-renesas-soc, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20200220130149.26283-1-geert+renesas@glider.be>

As GPIO hogs are configured at GPIO controller initialization time,
adding/removing GPIO hogs in DT overlays does not work.

Add support for GPIO hogs described in DT overlays by registering an OF
reconfiguration notifier, to handle the addition and removal of GPIO hog
subnodes to/from a GPIO controller device node.

Note that when a GPIO hog device node is being removed, its "gpios"
properties is no longer available, so we have to keep track of which
node a hog belongs to, which is done by adding a pointer to the hog's
device node to struct gpio_desc.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Drop RFC state,
  - Document that modifying existing gpio-hog nodes is not supported.
---
 drivers/gpio/gpiolib-of.c | 90 +++++++++++++++++++++++++++++++++++++++
 drivers/gpio/gpiolib-of.h |  2 +
 drivers/gpio/gpiolib.c    | 14 ++++--
 drivers/gpio/gpiolib.h    |  3 ++
 4 files changed, 106 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 2b47f93886075294..ccc449df3792ae97 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -628,6 +628,10 @@ static int of_gpiochip_add_hog(struct gpio_chip *chip, struct device_node *hog)
 		ret = gpiod_hog(desc, name, lflags, dflags);
 		if (ret < 0)
 			return ret;
+
+#ifdef CONFIG_OF_DYNAMIC
+		desc->hog = hog;
+#endif
 	}
 
 	return 0;
@@ -655,11 +659,97 @@ static int of_gpiochip_scan_gpios(struct gpio_chip *chip)
 			of_node_put(np);
 			return ret;
 		}
+
+		of_node_set_flag(np, OF_POPULATED);
 	}
 
 	return 0;
 }
 
+#ifdef CONFIG_OF_DYNAMIC
+/**
+ * of_gpiochip_remove_hog - Remove all hogs in a hog device node
+ * @chip:	gpio chip to act on
+ * @hog:	device node describing the hogs
+ */
+static void of_gpiochip_remove_hog(struct gpio_chip *chip,
+				   struct device_node *hog)
+{
+	struct gpio_desc *descs = chip->gpiodev->descs;
+	unsigned int i;
+
+	for (i = 0; i < chip->ngpio; i++) {
+		if (test_bit(FLAG_IS_HOGGED, &descs[i].flags) &&
+		    descs[i].hog == hog)
+			gpiochip_free_own_desc(&descs[i]);
+	}
+}
+
+static int of_gpiochip_match_node(struct gpio_chip *chip, void *data)
+{
+	return chip->gpiodev->dev.of_node == data;
+}
+
+static struct gpio_chip *of_find_gpiochip_by_node(struct device_node *np)
+{
+	return gpiochip_find(np, of_gpiochip_match_node);
+}
+
+static int of_gpio_notify(struct notifier_block *nb, unsigned long action,
+			  void *arg)
+{
+	struct of_reconfig_data *rd = arg;
+	struct gpio_chip *chip;
+	int ret;
+
+	/*
+	 * This only supports adding and removing complete gpio-hog nodes.
+	 * Modifying an existing gpio-hog node is not supported (except for
+	 * changing its "status" property, which is treated the same as
+	 * addition/removal).
+	 */
+	switch (of_reconfig_get_state_change(action, arg)) {
+	case OF_RECONFIG_CHANGE_ADD:
+		if (!of_property_read_bool(rd->dn, "gpio-hog"))
+			return NOTIFY_OK;	/* not for us */
+
+		if (of_node_test_and_set_flag(rd->dn, OF_POPULATED))
+			return NOTIFY_OK;
+
+		chip = of_find_gpiochip_by_node(rd->dn->parent);
+		if (chip == NULL)
+			return NOTIFY_OK;	/* not for us */
+
+		ret = of_gpiochip_add_hog(chip, rd->dn);
+		if (ret < 0) {
+			pr_err("%s: failed to add hogs for %pOF\n", __func__,
+			       rd->dn);
+			of_node_clear_flag(rd->dn, OF_POPULATED);
+			return notifier_from_errno(ret);
+		}
+		break;
+
+	case OF_RECONFIG_CHANGE_REMOVE:
+		if (!of_node_check_flag(rd->dn, OF_POPULATED))
+			return NOTIFY_OK;	/* already depopulated */
+
+		chip = of_find_gpiochip_by_node(rd->dn->parent);
+		if (chip == NULL)
+			return NOTIFY_OK;	/* not for us */
+
+		of_gpiochip_remove_hog(chip, rd->dn);
+		of_node_clear_flag(rd->dn, OF_POPULATED);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+struct notifier_block gpio_of_notifier = {
+	.notifier_call = of_gpio_notify,
+};
+#endif /* CONFIG_OF_DYNAMIC */
+
 /**
  * of_gpio_simple_xlate - translate gpiospec to the GPIO number and flags
  * @gc:		pointer to the gpio_chip structure
diff --git a/drivers/gpio/gpiolib-of.h b/drivers/gpio/gpiolib-of.h
index 9768831b1fe2f25b..ed26664f153782fc 100644
--- a/drivers/gpio/gpiolib-of.h
+++ b/drivers/gpio/gpiolib-of.h
@@ -35,4 +35,6 @@ static inline bool of_gpio_need_valid_mask(const struct gpio_chip *gc)
 }
 #endif /* CONFIG_OF_GPIO */
 
+extern struct notifier_block gpio_of_notifier;
+
 #endif /* GPIOLIB_OF_H */
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 56de871060ea211e..6f312220fe80acaf 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2925,6 +2925,9 @@ static bool gpiod_free_commit(struct gpio_desc *desc)
 		clear_bit(FLAG_PULL_DOWN, &desc->flags);
 		clear_bit(FLAG_BIAS_DISABLE, &desc->flags);
 		clear_bit(FLAG_IS_HOGGED, &desc->flags);
+#ifdef CONFIG_OF_DYNAMIC
+		desc->hog = NULL;
+#endif
 		ret = true;
 	}
 
@@ -5126,10 +5129,15 @@ static int __init gpiolib_dev_init(void)
 	if (ret < 0) {
 		pr_err("gpiolib: failed to allocate char dev region\n");
 		bus_unregister(&gpio_bus_type);
-	} else {
-		gpiolib_initialized = true;
-		gpiochip_setup_devs();
+		return ret;
 	}
+
+	gpiolib_initialized = true;
+	gpiochip_setup_devs();
+
+	if (IS_ENABLED(CONFIG_OF_DYNAMIC))
+		WARN_ON(of_reconfig_notifier_register(&gpio_of_notifier));
+
 	return ret;
 }
 core_initcall(gpiolib_dev_init);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 3e0aab2945d82974..18c75e83fd7679ec 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -119,6 +119,9 @@ struct gpio_desc {
 	const char		*label;
 	/* Name of the GPIO */
 	const char		*name;
+#ifdef CONFIG_OF_DYNAMIC
+	struct device_node	*hog;
+#endif
 };
 
 int gpiod_request(struct gpio_desc *desc, const char *label);
-- 
2.17.1


^ permalink raw reply related

* [PATCH v2 0/2] gpio: of: Add DT overlay support for GPIO hogs
From: Geert Uytterhoeven @ 2020-02-20 13:01 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Pantelis Antoniou,
	Frank Rowand, Rob Herring, Mark Rutland
  Cc: Peter Ujfalusi, Chris Brandt, linux-gpio, devicetree,
	linux-renesas-soc, linux-kernel, Geert Uytterhoeven

	Hi all,

As GPIO hogs are configured at GPIO controller initialization time,
adding/removing GPIO hogs in Device Tree overlays currently does not
work.  Hence this patch series adds support for that, by registering an
of_reconfig notifier, as is already done for platform, i2c, and SPI
devices.

Changes compared to v1[1]:
  - Drop RFC state,
  - Document that modifying existing gpio-hog nodes is not supported.

After I posted v1, Frank created a unittest[2] to demonstrate the
problem, and to verify to my series fixes it (thanks a lot!).

Thanks!

[1] "[PATCH/RFC 0/2] gpio: of: Add DT overlay support for GPIO hogs"
    https://lore.kernel.org/r/20191230133852.5890-1-geert+renesas@glider.be/
[2] "[RFC PATCH 0/2] of: unittest: add overlay gpio test to catch gpio hog
     problem"
    https://lore.kernel.org/r/1579070828-18221-1-git-send-email-frowand.list@gmail.com/

Geert Uytterhoeven (2):
  gpio: of: Extract of_gpiochip_add_hog()
  gpio: of: Add DT overlay support for GPIO hogs

 drivers/gpio/gpiolib-of.c | 139 +++++++++++++++++++++++++++++++++-----
 drivers/gpio/gpiolib-of.h |   2 +
 drivers/gpio/gpiolib.c    |  14 +++-
 drivers/gpio/gpiolib.h    |   3 +
 4 files changed, 139 insertions(+), 19 deletions(-)

-- 
2.17.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply

* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Double check bumping after the spinlock
From: Matthew Auld @ 2020-02-20 13:02 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx
In-Reply-To: <20200220123608.1666271-1-chris@chris-wilson.co.uk>

On 20/02/2020 12:36, Chris Wilson wrote:
> In preparation for making GEM execbuf parallel, we need to be prepared
> to handle very early declaration of dependencies -- even before our
> signaler has itself been submitted.
> 
> References: a79ca656b648 ("drm/i915: Push the wakeref->count deferral to the backend")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [arm64, debug] PTRACE_SINGLESTEP does not single-step a valid instruction
From: Mark Rutland @ 2020-02-20 13:02 UTC (permalink / raw)
  To: Will Deacon; +Cc: Luis Machado, linux-arm-kernel
In-Reply-To: <20200213120115.GD1405@willie-the-truck>

Hi Will, Luis,

On Thu, Feb 13, 2020 at 12:01:16PM +0000, Will Deacon wrote:
> Sorry for the very slow reply. I talked to Mark about this a bit but it
> seems that we never followed up here.
> 
> On Tue, Dec 10, 2019 at 05:00:18PM -0300, Luis Machado wrote:
> > Do you have any input regarding this particular situation?
> > 
> > It would be nice to get this fixed before the release of another GDB
> > version, if the fix is to live in GDB itself.
> 
> Basically, I'm very nervous about fixing this in the kernel because
> whatever we do will be visible to userspace. On the other hand, this
> part of the ptrace interface is only seriously used by GDB and we should
> make sure that it works well.
> 
> Does the diff below solve the problem? If so, can you confirm that it
> doesn't appear to regress anything else for GDB?
> 
> Cheers,
> 
> Will

> 
> --->8
> 
> diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
> index 7619f473155f..d825e3585e28 100644
> --- a/arch/arm64/include/asm/debug-monitors.h
> +++ b/arch/arm64/include/asm/debug-monitors.h
> @@ -109,6 +109,8 @@ void disable_debug_monitors(enum dbg_active_el el);
>  
>  void user_rewind_single_step(struct task_struct *task);
>  void user_fastforward_single_step(struct task_struct *task);
> +void user_regs_reset_single_step(struct user_pt_regs *regs,
> +				 struct task_struct *task);
>  
>  void kernel_enable_single_step(struct pt_regs *regs);
>  void kernel_disable_single_step(void);
> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
> index 48222a4760c2..7569deb1eac1 100644
> --- a/arch/arm64/kernel/debug-monitors.c
> +++ b/arch/arm64/kernel/debug-monitors.c
> @@ -141,17 +141,20 @@ postcore_initcall(debug_monitors_init);
>  /*
>   * Single step API and exception handling.
>   */
> -static void set_regs_spsr_ss(struct pt_regs *regs)
> +static void set_user_regs_spsr_ss(struct user_pt_regs *regs)
>  {
>  	regs->pstate |= DBG_SPSR_SS;
>  }
> -NOKPROBE_SYMBOL(set_regs_spsr_ss);
> +NOKPROBE_SYMBOL(set_user_regs_spsr_ss);
>  
> -static void clear_regs_spsr_ss(struct pt_regs *regs)
> +static void clear_user_regs_spsr_ss(struct user_pt_regs *regs)
>  {
>  	regs->pstate &= ~DBG_SPSR_SS;
>  }
> -NOKPROBE_SYMBOL(clear_regs_spsr_ss);
> +NOKPROBE_SYMBOL(clear_user_regs_spsr_ss);
> +
> +#define set_regs_spsr_ss(r)	set_user_regs_spsr_ss(&(r)->user_regs)
> +#define clear_regs_spsr_ss(r)	clear_user_regs_spsr_ss(&(r)->user_regs)
>  
>  static DEFINE_SPINLOCK(debug_hook_lock);
>  static LIST_HEAD(user_step_hook);
> @@ -404,6 +407,15 @@ void user_fastforward_single_step(struct task_struct *task)
>  		clear_regs_spsr_ss(task_pt_regs(task));
>  }
>  
> +void user_regs_reset_single_step(struct user_pt_regs *regs,
> +				 struct task_struct *task)
> +{
> +	if (test_tsk_thread_flag(task, TIF_SINGLESTEP))
> +		set_user_regs_spsr_ss(regs);
> +	else
> +		clear_user_regs_spsr_ss(regs);
> +}
> +
>  /* Kernel API */
>  void kernel_enable_single_step(struct pt_regs *regs)
>  {
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index cd6e5fa48b9c..d479fbcbd0d2 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -1934,8 +1934,8 @@ static int valid_native_regs(struct user_pt_regs *regs)
>   */
>  int valid_user_regs(struct user_pt_regs *regs, struct task_struct *task)
>  {
> -	if (!test_tsk_thread_flag(task, TIF_SINGLESTEP))
> -		regs->pstate &= ~DBG_SPSR_SS;
> +	/* https://lore.kernel.org/lkml/20191118131525.GA4180@willie-the-truck */
> +	user_regs_reset_single_step(regs, task);

I think this change means we do the right thing for signal entry/return
and ptrace messing with the regs. Instruction emulation seems to do the
right thing via skip_faulting_instruction().

I think there are a few more single-step edge cases lying around (e.g.
uprobes, rseq), but it looks like those have to be fixed separately. I
fear fixing uprobes might require a largler structural change to single
step, but ignoring uprobes the changes above seem to be sound.

If userspace doesn't consume the SS value today, I wonder if we should
hide it when dumping the SPSR to userspace, so that userspace has a
consistent view regardless of whether it's being stepped.

I'll try to dig into the uprobes stuff this afternoon, just in case that
needs us to do something substantially different.

The existing logic in valid_user_regs() doesn't make sense to me, given
SPSR_EL1.SS is immaterial unless MSCDR_EL1.SS == 1. I'm not sure if that
was overzealous or I've forgotten an edge case that we cared about in
the past.

>  
>  	if (is_compat_thread(task_thread_info(task)))
>  		return valid_compat_regs(regs);
> diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
> index 339882db5a91..bc54bdbfd760 100644
> --- a/arch/arm64/kernel/signal.c
> +++ b/arch/arm64/kernel/signal.c
> @@ -505,8 +505,12 @@ static int restore_sigframe(struct pt_regs *regs,
>  	forget_syscall(regs);
>  
>  	err |= !valid_user_regs(&regs->user_regs, current);
> -	if (err == 0)
> +
> +	if (err == 0) {
> +		/* Make it look like we stepped the sigreturn system call */
> +		user_fastforward_single_step(current);
>  		err = parse_user_sigframe(&user, sf);
> +	}

I don't understand this. AFAICT  we don't likewise for other SVCs, so
either I'm missing that, or there's something else I'm missing.

Why do we need to step sigreturn but not SVC generally?

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v4 05/16] s390x: protvirt: Handle diag 308 subcodes 0,1,3,4
From: Janosch Frank @ 2020-02-20 12:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-s390x, mihajlov, david, cohuck
In-Reply-To: <20200220125638.7241-1-frankja@linux.ibm.com>

As we now have access to the protection state of the cpus, we can
implement special handling of diag 308 subcodes for cpus in the
protected state.

For subcodes 0 and 1 we need to unshare all pages before continuing,
so the guest doesn't accidentally expose data when dumping.

For subcode 3/4 we tear down the protected VM and reboot into
unprotected mode. We do not provide a secure reboot.

Before we can do the unshare calls, we need to mark all cpus as
stopped.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 37 ++++++++++++++++++++++++++++++++++---
 target/s390x/diag.c        |  4 ++++
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 6fba8d9331..48dd3170ab 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -384,6 +384,20 @@ static void s390_machine_inject_pv_error(CPUState *cs)
     env->regs[r1 + 1] = 0xa02;
 }
 
+static void s390_pv_prepare_reset(CPUS390XState *env)
+{
+    CPUState *cs;
+
+    if (!env->pv) {
+        return;
+    }
+    CPU_FOREACH(cs) {
+        s390_cpu_set_state(S390_CPU_STATE_STOPPED, S390_CPU(cs));
+    }
+    s390_pv_unshare();
+    s390_pv_perf_clear_reset();
+}
+
 static void s390_machine_reset(MachineState *machine)
 {
     enum s390_reset reset_type;
@@ -391,6 +405,7 @@ static void s390_machine_reset(MachineState *machine)
     S390CPU *cpu;
     S390CcwMachineState *ms = S390_CCW_MACHINE(machine);
     static Error *local_err;
+    CPUS390XState *env;
 
     /* get the reset parameters, reset them once done */
     s390_ipl_get_reset_request(&cs, &reset_type);
@@ -399,10 +414,16 @@ static void s390_machine_reset(MachineState *machine)
     s390_cmma_reset();
 
     cpu = S390_CPU(cs);
+    env = &cpu->env;
 
     switch (reset_type) {
     case S390_RESET_EXTERNAL:
     case S390_RESET_REIPL:
+        if (ms->pv) {
+            s390_machine_unprotect(ms);
+            migrate_del_blocker(pv_mig_blocker);
+        }
+
         qemu_devices_reset();
         s390_crypto_reset();
 
@@ -410,21 +431,31 @@ static void s390_machine_reset(MachineState *machine)
         run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL);
         break;
     case S390_RESET_MODIFIED_CLEAR:
+        /*
+         * Susbsystem reset needs to be done before we unshare memory
+         * and loose access to VIRTIO structures in guest memory.
+         */
+        subsystem_reset();
+        s390_crypto_reset();
+        s390_pv_prepare_reset(env);
         CPU_FOREACH(t) {
             run_on_cpu(t, s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
         }
-        subsystem_reset();
-        s390_crypto_reset();
         run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
         break;
     case S390_RESET_LOAD_NORMAL:
+        /*
+         * Susbsystem reset needs to be done before we unshare memory
+         * and loose access to VIRTIO structures in guest memory.
+         */
+        subsystem_reset();
+        s390_pv_prepare_reset(env);
         CPU_FOREACH(t) {
             if (t == cs) {
                 continue;
             }
             run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
         }
-        subsystem_reset();
         run_on_cpu(cs, s390_do_cpu_initial_reset, RUN_ON_CPU_NULL);
         run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
         break;
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index 5e77d85652..aedc774695 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -68,6 +68,10 @@ int handle_diag_288(CPUS390XState *env, uint64_t r1, uint64_t r3)
 static int diag308_parm_check(CPUS390XState *env, uint64_t r1, uint64_t addr,
                               uintptr_t ra, bool write)
 {
+    /* Handled by the Ultravisor */
+    if (env->pv) {
+        return 0;
+    }
     if ((r1 & 1) || (addr & ~TARGET_PAGE_MASK)) {
         s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return -1;
-- 
2.20.1



^ permalink raw reply related

* [PATCH v4 08/16] s390x: protvirt: Move STSI data over SIDAD
From: Janosch Frank @ 2020-02-20 12:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-s390x, mihajlov, david, cohuck
In-Reply-To: <20200220125638.7241-1-frankja@linux.ibm.com>

For protected guests, we need to put the STSI emulation results into
the SIDA, so SIE will write them into the guest at the next entry.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 target/s390x/kvm.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index f222836df5..3a5a5146e3 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1795,11 +1795,16 @@ static int handle_tsch(S390CPU *cpu)
 
 static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
 {
+    CPUS390XState *env = &cpu->env;
     SysIB_322 sysib;
     int del;
 
-    if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib, sizeof(sysib))) {
-        return;
+    if (env->pv) {
+        s390_cpu_pv_mem_read(cpu, 0, &sysib, sizeof(sysib));
+    } else {
+        if (s390_cpu_virt_mem_read(cpu, addr, ar, &sysib, sizeof(sysib))) {
+            return;
+        }
     }
     /* Shift the stack of Extended Names to prepare for our own data */
     memmove(&sysib.ext_names[1], &sysib.ext_names[0],
@@ -1838,7 +1843,11 @@ static void insert_stsi_3_2_2(S390CPU *cpu, __u64 addr, uint8_t ar)
     /* Insert UUID */
     memcpy(sysib.vm[0].uuid, &qemu_uuid, sizeof(sysib.vm[0].uuid));
 
-    s390_cpu_virt_mem_write(cpu, addr, ar, &sysib, sizeof(sysib));
+    if (env->pv) {
+        s390_cpu_pv_mem_write(cpu, 0, &sysib, sizeof(sysib));
+    } else {
+        s390_cpu_virt_mem_write(cpu, addr, ar, &sysib, sizeof(sysib));
+    }
 }
 
 static int handle_stsi(S390CPU *cpu)
-- 
2.20.1



^ permalink raw reply related

* [PATCH v4 09/16] s390x: protvirt: SCLP interpretation
From: Janosch Frank @ 2020-02-20 12:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-s390x, mihajlov, david, cohuck
In-Reply-To: <20200220125638.7241-1-frankja@linux.ibm.com>

SCLP for a protected guest is done over the SIDAD, so we need to use
the s390_cpu_virt_mem_* functions to access the SIDAD instead of guest
memory when reading/writing SCBs.

To not confuse the sclp emulation, we set 0x4000 as the SCCB address,
since the function that injects the sclp external interrupt would
reject a zero sccb address.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 hw/s390x/sclp.c         | 17 +++++++++++++++++
 include/hw/s390x/sclp.h |  2 ++
 target/s390x/kvm.c      |  5 +++++
 3 files changed, 24 insertions(+)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index af0bfbc2ec..5136f5fcbe 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -193,6 +193,23 @@ static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, uint32_t code)
     }
 }
 
+#define SCLP_PV_DUMMY_ADDR 0x4000
+int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb,
+                                uint32_t code)
+{
+    SCLPDevice *sclp = get_sclp_device();
+    SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp);
+    SCCB work_sccb;
+    hwaddr sccb_len = sizeof(SCCB);
+
+    s390_cpu_pv_mem_read(env_archcpu(env), 0, &work_sccb, sccb_len);
+    sclp_c->execute(sclp, &work_sccb, code);
+    s390_cpu_pv_mem_write(env_archcpu(env), 0, &work_sccb,
+                          be16_to_cpu(work_sccb.h.length));
+    sclp_c->service_interrupt(sclp, SCLP_PV_DUMMY_ADDR);
+    return 0;
+}
+
 int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code)
 {
     SCLPDevice *sclp = get_sclp_device();
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index c54413b78c..c0a3faa37d 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -217,5 +217,7 @@ void s390_sclp_init(void);
 void sclp_service_interrupt(uint32_t sccb);
 void raise_irq_cpu_hotplug(void);
 int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);
+int sclp_service_call_protected(CPUS390XState *env, uint64_t sccb,
+                                uint32_t code);
 
 #endif
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 3a5a5146e3..31dd49729b 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1224,6 +1224,11 @@ static void kvm_sclp_service_call(S390CPU *cpu, struct kvm_run *run,
     sccb = env->regs[ipbh0 & 0xf];
     code = env->regs[(ipbh0 & 0xf0) >> 4];
 
+    if (run->s390_sieic.icptcode == ICPT_PV_INSTR) {
+        sclp_service_call_protected(env, sccb, code);
+        return;
+    }
+
     r = sclp_service_call(env, sccb, code);
     if (r < 0) {
         kvm_s390_program_interrupt(cpu, -r);
-- 
2.20.1



^ permalink raw reply related

* Re: [PATCH v3 09/37] KVM: s390: protvirt: Add initial vm and cpu lifecycle handling
From: David Hildenbrand @ 2020-02-20 13:02 UTC (permalink / raw)
  To: Christian Borntraeger, Janosch Frank
  Cc: KVM, Cornelia Huck, Thomas Huth, Ulrich Weigand, Claudio Imbrenda,
	linux-s390, Michael Mueller, Vasily Gorbik, Janosch Frank
In-Reply-To: <20200220104020.5343-10-borntraeger@de.ibm.com>


> +static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
> +{
> +	int r = 0;
> +	u16 dummy;
> +	void __user *argp = (void __user *)cmd->data;
> +
> +	switch (cmd->cmd) {
> +	case KVM_PV_ENABLE: {
> +		r = -EINVAL;
> +		if (kvm_s390_pv_is_protected(kvm))
> +			break;
> +
> +		r = kvm_s390_pv_alloc_vm(kvm);
> +		if (r)
> +			break;
> +
> +		/* FMT 4 SIE needs esca */
> +		r = sca_switch_to_extended(kvm);
> +		if (r) {
> +			kvm_s390_pv_dealloc_vm(kvm);
> +			kvm_s390_vcpu_unblock_all(kvm);

You forgot to remove that.

> +			mutex_unlock(&kvm->lock);

That's certainly wrong as well.

> +			break;
> +		}
> +		r = kvm_s390_pv_create_vm(kvm, &cmd->rc, &cmd->rrc);
> +		if (!r)
> +			r = kvm_s390_cpus_to_pv(kvm, &cmd->rc, &cmd->rrc);
> +		if (r)
> +			kvm_s390_pv_destroy_vm(kvm, &dummy, &dummy);

Should there be a kvm_s390_pv_dealloc_vm() as well?

> +
> +		break;
> +	}
> +	case KVM_PV_DISABLE: {
> +		r = -EINVAL;
> +		if (!kvm_s390_pv_is_protected(kvm))
> +			break;
> +
> +		kvm_s390_cpus_from_pv(kvm, &cmd->rc, &cmd->rrc);
> +		r = kvm_s390_pv_destroy_vm(kvm, &cmd->rc, &cmd->rrc);
> +		if (!r)
> +			kvm_s390_pv_dealloc_vm(kvm);

Hm, if destroy fails, the CPUs would already have been removed.

Is there a way to make kvm_s390_pv_destroy_vm() never fail? The return
value is always ignored except here ... which looks wrong.

> +		break;
> +	}

[...]

> @@ -2558,10 +2724,21 @@ static void kvm_free_vcpus(struct kvm *kvm)
>  
>  void kvm_arch_destroy_vm(struct kvm *kvm)
>  {
> +	u16 rc, rrc;
>  	kvm_free_vcpus(kvm);
>  	sca_dispose(kvm);
> -	debug_unregister(kvm->arch.dbf);
>  	kvm_s390_gisa_destroy(kvm);
> +	/*
> +	 * We are already at the end of life and kvm->lock is not taken.
> +	 * This is ok as the file descriptor is closed by now and nobody
> +	 * can mess with the pv state. To avoid lockdep_assert_held from
> +	 * complaining we do not use kvm_s390_pv_is_protected.
> +	 */
> +	if (kvm_s390_pv_get_handle(kvm)) {

I'd prefer something like kvm_s390_pv_is_protected_unlocked(), but I
guess for these few use cases, this is fine.


> +		kvm_s390_pv_destroy_vm(kvm, &rc, &rrc);
> +		kvm_s390_pv_dealloc_vm(kvm);
> +	}
> +	debug_unregister(kvm->arch.dbf);
>  	free_page((unsigned long)kvm->arch.sie_page2);
>  	if (!kvm_is_ucontrol(kvm))
>  		gmap_remove(kvm->arch.gmap);
> @@ -2657,6 +2834,9 @@ static int sca_switch_to_extended(struct kvm *kvm)
>  	unsigned int vcpu_idx;
>  	u32 scaol, scaoh;
>  
> +	if (kvm->arch.use_esca)
> +		return 0;
> +
>  	new_sca = alloc_pages_exact(sizeof(*new_sca), GFP_KERNEL|__GFP_ZERO);
>  	if (!new_sca)
>  		return -ENOMEM;
> @@ -2908,6 +3088,7 @@ static void kvm_s390_vcpu_setup_model(struct kvm_vcpu *vcpu)
>  static int kvm_s390_vcpu_setup(struct kvm_vcpu *vcpu)
>  {
>  	int rc = 0;
> +	u16 uvrc, uvrrc;
>  
>  	atomic_set(&vcpu->arch.sie_block->cpuflags, CPUSTAT_ZARCH |
>  						    CPUSTAT_SM |
> @@ -2975,6 +3156,11 @@ static int kvm_s390_vcpu_setup(struct kvm_vcpu *vcpu)
>  
>  	kvm_s390_vcpu_crypto_setup(vcpu);
>  
> +	mutex_lock(&vcpu->kvm->lock);
> +	if (kvm_s390_pv_is_protected(vcpu->kvm))
> +		rc = kvm_s390_pv_create_cpu(vcpu, &uvrc, &uvrrc);
> +	mutex_unlock(&vcpu->kvm->lock);

Do we have to cleanup anything? (e.g., cmma page) I *think*
kvm_arch_vcpu_destroy() is not called when kvm_arch_vcpu_create() fails ...

> +
>  	return rc;
>  }
>  
> diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
> index 83dabb18e4d9..d62de29b2d6c 100644
> --- a/arch/s390/kvm/kvm-s390.h
> +++ b/arch/s390/kvm/kvm-s390.h
> @@ -15,6 +15,7 @@
>  #include <linux/hrtimer.h>
>  #include <linux/kvm.h>
>  #include <linux/kvm_host.h>
> +#include <linux/lockdep.h>
>  #include <asm/facility.h>
>  #include <asm/processor.h>
>  #include <asm/sclp.h>
> @@ -207,6 +208,40 @@ static inline int kvm_s390_user_cpu_state_ctrl(struct kvm *kvm)
>  	return kvm->arch.user_cpu_state_ctrl != 0;
>  }
>  
> +/* implemented in pv.c */
> +void kvm_s390_pv_dealloc_vm(struct kvm *kvm);
> +int kvm_s390_pv_alloc_vm(struct kvm *kvm);
> +int kvm_s390_pv_create_vm(struct kvm *kvm, u16 *rc, u16 *rrc);
> +int kvm_s390_pv_create_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc);
> +int kvm_s390_pv_destroy_vm(struct kvm *kvm, u16 *rc, u16 *rrc);
> +void kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc);
> +int kvm_s390_pv_set_sec_parms(struct kvm *kvm, void *hdr, u64 length, u16 *rc,
> +			      u16 *rrc);
> +int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
> +		       unsigned long tweak, u16 *rc, u16 *rrc);
> +
> +static inline u64 kvm_s390_pv_get_handle(struct kvm *kvm)
> +{
> +	return kvm->arch.pv.handle;
> +}
> +
> +static inline u64 kvm_s390_pv_cpu_get_handle(struct kvm_vcpu *vcpu)
> +{
> +	return vcpu->arch.pv.handle;
> +}
> +
> +static inline bool kvm_s390_pv_is_protected(struct kvm *kvm)

Could have been "kvm_s390_is_protected" or "kvm_s390_is_pv", but also
fine with me. (maybe I even suggested that one without caring about that
detail :) )

[...]

> +
>  /* implemented in interrupt.c */
>  int kvm_s390_handle_wait(struct kvm_vcpu *vcpu);
>  void kvm_s390_vcpu_wakeup(struct kvm_vcpu *vcpu);
> diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
> new file mode 100644
> index 000000000000..67ea9a18ed8f
> --- /dev/null
> +++ b/arch/s390/kvm/pv.c
> @@ -0,0 +1,256 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Hosting Secure Execution virtual machines
> + *
> + * Copyright IBM Corp. 2019
> + *    Author(s): Janosch Frank <frankja@linux.ibm.com>

I'd assume you're an author as well at this point ;)

[...]

> +
> +int kvm_s390_pv_set_sec_parms(struct kvm *kvm, void *hdr, u64 length, u16 *rc,
> +			      u16 *rrc)
> +{
> +	struct uv_cb_ssc uvcb = {
> +		.header.cmd = UVC_CMD_SET_SEC_CONF_PARAMS,
> +		.header.len = sizeof(uvcb),
> +		.sec_header_origin = (u64)hdr,
> +		.sec_header_len = length,
> +		.guest_handle = kvm_s390_pv_get_handle(kvm),
> +	};
> +	int cc;
> +
> +	cc = uv_call(0, (u64)&uvcb);

int cc = ... could be done.


> +	*rc = uvcb.header.rc;
> +	*rrc = uvcb.header.rrc;
> +	KVM_UV_EVENT(kvm, 3, "PROTVIRT VM SET PARMS: rc %x rrc %x",
> +		     *rc, *rrc);
> +	if (cc)
> +		return -EINVAL;
> +	return 0;
> +}
> +
> +static int unpack_one(struct kvm *kvm, unsigned long addr, u64 tweak[2],
> +		      u16 *rc, u16 *rrc)
> +{
> +	struct uv_cb_unp uvcb = {
> +		.header.cmd = UVC_CMD_UNPACK_IMG,
> +		.header.len = sizeof(uvcb),
> +		.guest_handle = kvm_s390_pv_get_handle(kvm),
> +		.gaddr = addr,
> +		.tweak[0] = tweak[0],
> +		.tweak[1] = tweak[1],
> +	};
> +	int ret;
> +
> +	ret = gmap_make_secure(kvm->arch.gmap, addr, &uvcb);

... similarly, with ret.

> +	*rc = uvcb.header.rc;
> +	*rrc = uvcb.header.rrc;
> +
> +	if (ret && ret != -EAGAIN)
> +		KVM_UV_EVENT(kvm, 3, "PROTVIRT VM UNPACK: failed addr %llx with rc %x rrc %x",
> +			     uvcb.gaddr, *rc, *rrc);
> +	return ret;
> +}
> +
> +int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
> +		       unsigned long tweak, u16 *rc, u16 *rrc)
> +{
> +	u64 tw[2] = {tweak, 0};

I have no idea what tweaks are in this context. So I have to trust you
guys on the implementation, because I don't understand it.

Especially, why can't we simply have

s/tweak/tweak/

offset = 0;

while (offset < size) {
	...
	ret = unpack_one(kvm, addr, tweak, offset, rc, rrc);
				    ^ no idea what tweak is
	...
	... offset +=  PAGE_SIZE;
}

But maybe I am missing what the whole array is about.

> +	int ret = 0;
> +
> +	if (addr & ~PAGE_MASK || !size || size & ~PAGE_MASK)
> +		return -EINVAL;
> +
> +	KVM_UV_EVENT(kvm, 3, "PROTVIRT VM UNPACK: start addr %lx size %lx",
> +		     addr, size);
> +
> +	while (tw[1] < size) {> +		ret = unpack_one(kvm, addr, tw, rc, rrc);
> +		if (ret == -EAGAIN) {
> +			cond_resched();
> +			if (fatal_signal_pending(current))
> +				break;
> +			continue;
> +		}
> +		if (ret)
> +			break;
> +		addr += PAGE_SIZE;
> +		tw[1] += PAGE_SIZE;
> +	}
> +	if (!ret)
> +		KVM_UV_EVENT(kvm, 3, "%s", "PROTVIRT VM UNPACK: successful");
> +	return ret;
> +}

[...]
> +enum pv_cmd_id {
> +	KVM_PV_ENABLE,
> +	KVM_PV_DISABLE,
> +	KVM_PV_VM_SET_SEC_PARMS,
> +	KVM_PV_VM_UNPACK,
> +	KVM_PV_VM_VERIFY,

I wonder if we should just drop "_VM" from all of these ...

[...]


-- 
Thanks,

David / dhildenb

^ permalink raw reply

* [PATCH v4 16/16] docs: Add protvirt docs
From: Janosch Frank @ 2020-02-20 12:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-s390x, mihajlov, david, cohuck
In-Reply-To: <20200220125638.7241-1-frankja@linux.ibm.com>

Lets add some documentation for the Protected VM functionality.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 docs/protvirt.rst | 53 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 docs/protvirt.rst

diff --git a/docs/protvirt.rst b/docs/protvirt.rst
new file mode 100644
index 0000000000..8bfa72be01
--- /dev/null
+++ b/docs/protvirt.rst
@@ -0,1 +1,53 @@
+Protected Virtualization on s390x
+========================
+The memory and most of the register contents of Protected Virtual
+Machines (PVMs) are inaccessible to the hypervisor, effectively
+prohibiting VM introspection when the VM is running. At rest, PVMs are
+encrypted and can only be decrypted by the firmware of specific IBM Z
+machines.
+
+
+Prerequisites
+-------------
+To run PVMs, you need to have a machine with the Protected
+Virtualization feature, which is indicated by the Ultravisor Call
+facility (stfle bit 158). This is a KVM only feature, therefore you
+need a KVM which is able to support PVMs and activate the Ultravisor
+initialization by setting "prot_virt=1" on the kernel command line.
+
+If those requirements are met, the capability "KVM_CAP_S390_PROTECTED"
+will indicate that KVM can support PVMs on that LPAR.
+
+
+QEMU Settings
+-------------
+To indicate to the VM that it can move into protected mode, the
+"Unpack facility" (stfle bit 161) needs to be part of the cpu model of
+the VM.
+
+All I/O devices need to use the IOMMU.
+Passthrough devices are currently not supported.
+
+Host huge page backings are not supported. The guest however can use
+huge pages as indicated by its facilities.
+
+
+Boot Process
+-----------------
+A secure guest image can be booted from disk and using the QEMU
+command line. Booting from disk is done by the unmodified s390-ccw
+BIOS. I.e., the bootmap is interpreted and a number of components is
+read into memory and control is transferred to one of the components
+(zipl stage3), which does some fixups and then transfers control to
+some program residing in guest memory, which is normally the OS
+kernel. The secure image has another component prepended (stage3a)
+which uses the new diag308 subcodes 8 and 10 to trigger the transition
+into secure mode.
+
+Booting from the command line requires that the file passed
+via -kernel has the same memory layout as would result from the disk
+boot. This memory layout includes the encrypted components (kernel,
+initrd, cmdline), the stage3a loader and metadata. In case this boot
+method is used, the command line options -initrd and -cmdline are
+ineffective.  The preparation of secure guest image is done by a
+program (name tbd) of the s390-tools package.
-- 
2.20.1



^ permalink raw reply related

* [PATCH v4 04/16] s390x: protvirt: Add migration blocker
From: Janosch Frank @ 2020-02-20 12:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-s390x, mihajlov, david, cohuck
In-Reply-To: <20200220125638.7241-1-frankja@linux.ibm.com>

Migration is not yet supported.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index aa974d294e..6fba8d9331 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -42,6 +42,9 @@
 #include "hw/s390x/tod.h"
 #include "sysemu/sysemu.h"
 #include "hw/s390x/pv.h"
+#include "migration/blocker.h"
+
+static Error *pv_mig_blocker;
 
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)
 {
@@ -387,6 +390,7 @@ static void s390_machine_reset(MachineState *machine)
     CPUState *cs, *t;
     S390CPU *cpu;
     S390CcwMachineState *ms = S390_CCW_MACHINE(machine);
+    static Error *local_err;
 
     /* get the reset parameters, reset them once done */
     s390_ipl_get_reset_request(&cs, &reset_type);
@@ -436,13 +440,20 @@ static void s390_machine_reset(MachineState *machine)
         }
         run_on_cpu(cs, s390_do_cpu_reset, RUN_ON_CPU_NULL);
 
-        if (s390_machine_protect(ms)) {
+        if (!pv_mig_blocker) {
+            error_setg(&pv_mig_blocker,
+                       "protected VMs are currently not migrateable.");
+        }
+        migrate_add_blocker(pv_mig_blocker, &local_err);
+        if (!local_err && s390_machine_protect(ms)) {
             s390_machine_inject_pv_error(cs);
-            s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
-            return;
+            migrate_del_blocker(pv_mig_blocker);
+            goto pv_err;
         }
 
         run_on_cpu(cs, s390_do_cpu_load_normal, RUN_ON_CPU_NULL);
+pv_err:
+        s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
         break;
     default:
         g_assert_not_reached();
-- 
2.20.1



^ permalink raw reply related

* [PATCH v4 11/16] s390x: protvirt: Move diag 308 data over SIDAD
From: Janosch Frank @ 2020-02-20 12:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-s390x, mihajlov, david, cohuck
In-Reply-To: <20200220125638.7241-1-frankja@linux.ibm.com>

For protected guests the IPIB is written/read to/from the satellite
block, so we need those accesses to go through
s390_cpu_pv_mem_read/write().

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 target/s390x/diag.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index aedc774695..be80c0367b 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -88,6 +88,7 @@ static int diag308_parm_check(CPUS390XState *env, uint64_t r1, uint64_t addr,
 void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
 {
     CPUState *cs = env_cpu(env);
+    S390CPU *cpu = S390_CPU(cs);
     uint64_t addr =  env->regs[r1];
     uint64_t subcode = env->regs[r3];
     IplParameterBlock *iplb;
@@ -119,13 +120,22 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
             return;
         }
         iplb = g_new0(IplParameterBlock, 1);
-        cpu_physical_memory_read(addr, iplb, sizeof(iplb->len));
+        if (!env->pv) {
+            cpu_physical_memory_read(addr, iplb, sizeof(iplb->len));
+        } else {
+            s390_cpu_pv_mem_read(cpu, 0, iplb, sizeof(iplb->len));
+        }
+
         if (!iplb_valid_len(iplb)) {
             env->regs[r1 + 1] = DIAG_308_RC_INVALID;
             goto out;
         }
 
-        cpu_physical_memory_read(addr, iplb, be32_to_cpu(iplb->len));
+        if (!env->pv) {
+            cpu_physical_memory_read(addr, iplb, be32_to_cpu(iplb->len));
+        } else {
+            s390_cpu_pv_mem_read(cpu, 0, iplb, be32_to_cpu(iplb->len));
+        }
 
         if (!iplb_valid_ccw(iplb) && !iplb_valid_fcp(iplb) &&
             !(iplb_valid_pv(iplb) && s390_ipl_pv_check_components(iplb) >= 0)) {
@@ -137,7 +147,7 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
         env->regs[r1 + 1] = DIAG_308_RC_OK;
 out:
         g_free(iplb);
-        return;
+        break;
     case DIAG308_STORE:
     case DIAG308_PV_STORE:
         if (diag308_parm_check(env, r1, addr, ra, true)) {
@@ -148,12 +158,18 @@ out:
         } else {
             iplb = s390_ipl_get_iplb();
         }
-        if (iplb) {
-            cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len));
-            env->regs[r1 + 1] = DIAG_308_RC_OK;
-        } else {
+        if (!iplb) {
             env->regs[r1 + 1] = DIAG_308_RC_NO_CONF;
+            return;
         }
+
+        if (!env->pv) {
+            cpu_physical_memory_write(addr, iplb, be32_to_cpu(iplb->len));
+        } else {
+            s390_cpu_pv_mem_write(cpu, 0, iplb, be32_to_cpu(iplb->len));
+        }
+
+        env->regs[r1 + 1] = DIAG_308_RC_OK;
         break;
     case DIAG308_PV_START:
         iplb = s390_ipl_get_iplb_secure();
-- 
2.20.1



^ permalink raw reply related

* [PATCH v4 12/16] s390x: protvirt: Disable address checks for PV guest IO emulation
From: Janosch Frank @ 2020-02-20 12:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-s390x, mihajlov, david, cohuck
In-Reply-To: <20200220125638.7241-1-frankja@linux.ibm.com>

IO instruction data is routed through SIDAD for protected guests, so
adresses do not need to be checked, as this is kernel memory.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 target/s390x/ioinst.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index c437a1d8c6..e4102430aa 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -17,6 +17,16 @@
 #include "trace.h"
 #include "hw/s390x/s390-pci-bus.h"
 
+static uint64_t get_address_from_regs(CPUS390XState *env, uint32_t ipb,
+                                      uint8_t *ar)
+{
+    if (env->pv) {
+        *ar = 0;
+        return 0;
+    }
+    return decode_basedisp_s(env, ipb, ar);
+}
+
 int ioinst_disassemble_sch_ident(uint32_t value, int *m, int *cssid, int *ssid,
                                  int *schid)
 {
@@ -114,7 +124,7 @@ void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
     CPUS390XState *env = &cpu->env;
     uint8_t ar;
 
-    addr = decode_basedisp_s(env, ipb, &ar);
+    addr = get_address_from_regs(env, ipb, &ar);
     if (addr & 3) {
         s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return;
@@ -171,7 +181,7 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
     CPUS390XState *env = &cpu->env;
     uint8_t ar;
 
-    addr = decode_basedisp_s(env, ipb, &ar);
+    addr = get_address_from_regs(env, ipb, &ar);
     if (addr & 3) {
         s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return;
@@ -203,7 +213,7 @@ void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
     CPUS390XState *env = &cpu->env;
     uint8_t ar;
 
-    addr = decode_basedisp_s(env, ipb, &ar);
+    addr = get_address_from_regs(env, ipb, &ar);
     if (addr & 3) {
         s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return;
@@ -234,7 +244,7 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb,
     CPUS390XState *env = &cpu->env;
     uint8_t ar;
 
-    addr = decode_basedisp_s(env, ipb, &ar);
+    addr = get_address_from_regs(env, ipb, &ar);
     if (addr & 3) {
         s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return;
@@ -303,7 +313,7 @@ int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
         return -EIO;
     }
     trace_ioinst_sch_id("tsch", cssid, ssid, schid);
-    addr = decode_basedisp_s(env, ipb, &ar);
+    addr = get_address_from_regs(env, ipb, &ar);
     if (addr & 3) {
         s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return -EIO;
@@ -601,7 +611,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
 {
     ChscReq *req;
     ChscResp *res;
-    uint64_t addr;
+    uint64_t addr = 0;
     int reg;
     uint16_t len;
     uint16_t command;
@@ -610,7 +620,9 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
 
     trace_ioinst("chsc");
     reg = (ipb >> 20) & 0x00f;
-    addr = env->regs[reg];
+    if (!env->pv) {
+        addr = env->regs[reg];
+    }
     /* Page boundary? */
     if (addr & 0xfff) {
         s390_program_interrupt(env, PGM_SPECIFICATION, ra);
-- 
2.20.1



^ permalink raw reply related

* Re: [PATCH v5 01/10] capabilities: introduce CAP_PERFMON to kernel and user space
From: Alexey Budankov @ 2020-02-20 13:05 UTC (permalink / raw)
  To: Thomas Gleixner, Stephen Smalley, Serge Hallyn, James Morris
  Cc: Mark Rutland, Song Liu, Peter Zijlstra, benh@kernel.crashing.org,
	Will Deacon, Alexei Starovoitov, Paul Mackerras, Jiri Olsa,
	Alexei Starovoitov, Andi Kleen, Michael Ellerman, Igor Lubashev,
	Alexander Shishkin, Ingo Molnar, oprofile-list, linux-arm-kernel,
	Robert Richter, selinux@vger.kernel.org,
	intel-gfx@lists.freedesktop.org, Arnaldo Carvalho de Melo,
	Namhyung Kim, Stephane
In-Reply-To: <7d6f4210-423f-e454-3910-9f8e17dff1aa@linux.intel.com>


On 07.02.2020 16:39, Alexey Budankov wrote:
> 
> On 07.02.2020 14:38, Thomas Gleixner wrote:
>> Alexey Budankov <alexey.budankov@linux.intel.com> writes:
>>> On 22.01.2020 17:25, Alexey Budankov wrote:
>>>> On 22.01.2020 17:07, Stephen Smalley wrote:
>>>>>> It keeps the implementation simple and readable. The implementation is more
>>>>>> performant in the sense of calling the API - one capable() call for CAP_PERFMON
>>>>>> privileged process.
>>>>>>
>>>>>> Yes, it bloats audit log for CAP_SYS_ADMIN privileged and unprivileged processes,
>>>>>> but this bloating also advertises and leverages using more secure CAP_PERFMON
>>>>>> based approach to use perf_event_open system call.
>>>>>
>>>>> I can live with that.  We just need to document that when you see
>>>>> both a CAP_PERFMON and a CAP_SYS_ADMIN audit message for a process,
>>>>> try only allowing CAP_PERFMON first and see if that resolves the
>>>>> issue.  We have a similar issue with CAP_DAC_READ_SEARCH versus
>>>>> CAP_DAC_OVERRIDE.
>>>>
>>>> perf security [1] document can be updated, at least, to align and document 
>>>> this audit logging specifics.
>>>
>>> And I plan to update the document right after this patch set is accepted.
>>> Feel free to let me know of the places in the kernel docs that also
>>> require update w.r.t CAP_PERFMON extension.
>>
>> The documentation update wants be part of the patch set and not planned
>> to be done _after_ the patch set is merged.
> 
> Well, accepted. It is going to make patches #11 and beyond.

Patches #11 and #12 of v7 [1] contain information on CAP_PERFMON intention and usage.
Patch for man-pages [2] extends perf_event_open.2 documentation.

Thanks,
Alexey

---
[1] https://lore.kernel.org/lkml/c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com/
[2] https://lore.kernel.org/lkml/18d1083d-efe5-f5f8-c531-d142c0e5c1a8@linux.intel.com/

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH v3 10/37] KVM: s390: protvirt: Add KVM api documentation
From: David Hildenbrand @ 2020-02-20 13:05 UTC (permalink / raw)
  To: Christian Borntraeger, Janosch Frank
  Cc: KVM, Cornelia Huck, Thomas Huth, Ulrich Weigand, Claudio Imbrenda,
	linux-s390, Michael Mueller, Vasily Gorbik, Janosch Frank
In-Reply-To: <20200220104020.5343-11-borntraeger@de.ibm.com>

On 20.02.20 11:39, Christian Borntraeger wrote:
> From: Janosch Frank <frankja@linux.ibm.com>
> 
> Add documentation for KVM_CAP_S390_PROTECTED capability and the
> KVM_S390_PV_COMMAND and KVM_S390_PV_COMMAND_VCPU ioctls.

Outdated.

I suggest moving this after "[PATCH v3 37/37] KVM: s390: protvirt:
introduce and enable KVM_CAP_S390_PROTECTED" or even squashing it into
that one.

> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> [borntraeger@de.ibm.com: patch merging, splitting, fixing]
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  Documentation/virt/kvm/api.rst | 52 ++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 97a72a53fa4b..77e1edfe5d4c 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -4646,6 +4646,51 @@ the clear cpu reset definition in the POP. However, the cpu is not put
>  into ESA mode. This reset is a superset of the initial reset.
>  
>  
> +4.125 KVM_S390_PV_COMMAND
> +-------------------------
> +
> +:Capability: KVM_CAP_S390_PROTECTED
> +:Architectures: s390
> +:Type: vm ioctl
> +:Parameters: struct kvm_pv_cmd
> +:Returns: 0 on success, < 0 on error
> +
> +::
> +
> +  struct kvm_pv_cmd {
> +	__u32 cmd;	/* Command to be executed */
> +	__u16 rc;	/* Ultravisor return code */
> +	__u16 rrc;	/* Ultravisor return reason code */
> +	__u64 data;	/* Data or address */
> +	__u32 flags;    /* flags for future extensions. Must be 0 for now */
> +	__u32 reserved[3];
> +  };
> +
> +cmd values:
> +
> +KVM_PV_ENABLE
> +  Allocate memory and register the VM with the Ultravisor, thereby
> +  donating memory to the Ultravisor making it inaccessible to KVM.
> +  Also converts all existing CPUs to protected ones. Future hotplug
> +  CPUs will become protected during creation.
> +
> +KVM_PV_DISABLE
> +  Deregisters the VM from the Ultravisor and frees memory that was
> +  donated, so the kernel can use it again. All registered VCPUs are
> +  converted back to non-protected ones.
> +
> +KVM_PV_VM_SET_SEC_PARMS
> +  Pass the image header from VM memory to the Ultravisor in
> +  preparation of image unpacking and verification.
> +
> +KVM_PV_VM_UNPACK
> +  Unpack (protect and decrypt) a page of the encrypted boot image.
> +
> +KVM_PV_VM_VERIFY
> +  Verify the integrity of the unpacked image. Only if this succeeds,
> +  KVM is allowed to start protected VCPUs.
> +
> +
>  5. The kvm_run structure
>  ========================
>  
> @@ -6024,3 +6069,10 @@ Architectures: s390
>  
>  This capability indicates that the KVM_S390_NORMAL_RESET and
>  KVM_S390_CLEAR_RESET ioctls are available.
> +
> +8.23 KVM_CAP_S390_PROTECTED
> +
> +Architecture: s390
> +
> +This capability indicates that KVM can start protected VMs and the
> +Ultravisor has therefore been initialized.
> 

Maybe document that KVM_S390_PV_COMMAND and the new MP is available.
Also maybe that MP commands can now fail.

-- 
Thanks,

David / dhildenb

^ permalink raw reply

* Re: [docs] "KBUILD_DEFCONFIG" description in kernel-dev manual seems wrong
From: Bruce Ashfield @ 2020-02-20 13:05 UTC (permalink / raw)
  To: docs
In-Reply-To: <alpine.LFD.2.21.2002200346570.15458@localhost.localdomain>

On Thu, Feb 20, 2020 at 3:53 AM rpjday@crashcourse.ca
<rpjday@crashcourse.ca> wrote:
>
>
>   in current kernel-dev manual, section on "in-tree" defconfig file:
>
> https://www.yoctoproject.org/docs/current/kernel-dev/kernel-dev.html#using-an-in-tree-defconfig-file
>
> the example given is:
>
>  KBUILD_DEFCONFIG_common-pc ?= "/home/scottrif/configfiles/my_defconfig_file"
>
> but that's not an "in-tree" file, is it? i thought that variable
> referred to just the file name within an existing kernel source tree,
> no? the processing in kernel-yocto.bbclass certainly suggests that:

Correct. It is for defconfigs that are shipped with the kernel itself.
The standard 'defconfig' in the SRC_URI covers other cases.

Bruce

>
>  if [ -f "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}" ]; then
>
> should i submit a patch for that? perhaps steal the example from the
> reference manual:
>
>   KBUILD_DEFCONFIG_raspberrypi2 = "bcm2709_defconfig"
>
> rday
> 



-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II

^ permalink raw reply

* Re: [Intel-gfx] [PATCH v5 01/10] capabilities: introduce CAP_PERFMON to kernel and user space
From: Alexey Budankov @ 2020-02-20 13:05 UTC (permalink / raw)
  To: Thomas Gleixner, Stephen Smalley, Serge Hallyn, James Morris
  Cc: Mark Rutland, Song Liu, Peter Zijlstra, benh@kernel.crashing.org,
	Will Deacon, Alexei Starovoitov, Paul Mackerras, Jiri Olsa,
	Alexei Starovoitov, Andi Kleen, Michael Ellerman, Igor Lubashev,
	Alexander Shishkin, Ingo Molnar, oprofile-list, linux-arm-kernel,
	Robert Richter, selinux@vger.kernel.org,
	intel-gfx@lists.freedesktop.org, Arnaldo Carvalho de Melo,
	Namhyung Kim, Stephane Eranian, linux-parisc@vger.kernel.org,
	linux-kernel, Andy Lutomirski, linux-perf-users@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <7d6f4210-423f-e454-3910-9f8e17dff1aa@linux.intel.com>


On 07.02.2020 16:39, Alexey Budankov wrote:
> 
> On 07.02.2020 14:38, Thomas Gleixner wrote:
>> Alexey Budankov <alexey.budankov@linux.intel.com> writes:
>>> On 22.01.2020 17:25, Alexey Budankov wrote:
>>>> On 22.01.2020 17:07, Stephen Smalley wrote:
>>>>>> It keeps the implementation simple and readable. The implementation is more
>>>>>> performant in the sense of calling the API - one capable() call for CAP_PERFMON
>>>>>> privileged process.
>>>>>>
>>>>>> Yes, it bloats audit log for CAP_SYS_ADMIN privileged and unprivileged processes,
>>>>>> but this bloating also advertises and leverages using more secure CAP_PERFMON
>>>>>> based approach to use perf_event_open system call.
>>>>>
>>>>> I can live with that.  We just need to document that when you see
>>>>> both a CAP_PERFMON and a CAP_SYS_ADMIN audit message for a process,
>>>>> try only allowing CAP_PERFMON first and see if that resolves the
>>>>> issue.  We have a similar issue with CAP_DAC_READ_SEARCH versus
>>>>> CAP_DAC_OVERRIDE.
>>>>
>>>> perf security [1] document can be updated, at least, to align and document 
>>>> this audit logging specifics.
>>>
>>> And I plan to update the document right after this patch set is accepted.
>>> Feel free to let me know of the places in the kernel docs that also
>>> require update w.r.t CAP_PERFMON extension.
>>
>> The documentation update wants be part of the patch set and not planned
>> to be done _after_ the patch set is merged.
> 
> Well, accepted. It is going to make patches #11 and beyond.

Patches #11 and #12 of v7 [1] contain information on CAP_PERFMON intention and usage.
Patch for man-pages [2] extends perf_event_open.2 documentation.

Thanks,
Alexey

---
[1] https://lore.kernel.org/lkml/c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com/
[2] https://lore.kernel.org/lkml/18d1083d-efe5-f5f8-c531-d142c0e5c1a8@linux.intel.com/

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* [PATCH] x86: Correct error return value in mrccache_get_region()
From: Bin Meng @ 2020-02-20 13:05 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <CAEUhbmUzUFq9mDrShUK9zjdfEKJUOAOUOQbVr=LT6_qwRCfpDQ@mail.gmail.com>

On Mon, Feb 3, 2020 at 8:44 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Mon, Feb 3, 2020 at 4:37 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > This function doesn't use uclass_find_first_device() correctly. Add a
> > check that the device is found so we don't try to read properties from a
> > NULL device.
> >
> > The fixes booting on minnoxmax.
> >
> > Fixes: 87f1084a630 ("x86: Adjust mrccache_get_region() to use livetree")
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  arch/x86/lib/mrccache.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86, thanks!

^ permalink raw reply

* Re: [PATCH v5 01/10] capabilities: introduce CAP_PERFMON to kernel and user space
From: Alexey Budankov @ 2020-02-20 13:05 UTC (permalink / raw)
  To: Thomas Gleixner, Stephen Smalley, Serge Hallyn, James Morris
  Cc: Alexei Starovoitov, Peter Zijlstra, Arnaldo Carvalho de Melo,
	Ingo Molnar, jani.nikula@linux.intel.com,
	joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com,
	benh@kernel.crashing.org, Paul Mackerras, Michael Ellerman,
	Will Deacon, Mark Rutland, Robert Richter, Alexei Starovoitov,
	Jiri Olsa, Andi Kleen, Stephane Eranian, Igor Lubashev,
	Alexander Shishkin, Namhyung Kim, Song Liu, Lionel Landwerlin,
	linux-kernel, linux-security-module@vger.kernel.org,
	selinux@vger.kernel.org, intel-gfx@lists.freedesktop.org,
	linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel, linux-perf-users@vger.kernel.org, oprofile-list,
	Andy Lutomirski
In-Reply-To: <7d6f4210-423f-e454-3910-9f8e17dff1aa@linux.intel.com>


On 07.02.2020 16:39, Alexey Budankov wrote:
> 
> On 07.02.2020 14:38, Thomas Gleixner wrote:
>> Alexey Budankov <alexey.budankov@linux.intel.com> writes:
>>> On 22.01.2020 17:25, Alexey Budankov wrote:
>>>> On 22.01.2020 17:07, Stephen Smalley wrote:
>>>>>> It keeps the implementation simple and readable. The implementation is more
>>>>>> performant in the sense of calling the API - one capable() call for CAP_PERFMON
>>>>>> privileged process.
>>>>>>
>>>>>> Yes, it bloats audit log for CAP_SYS_ADMIN privileged and unprivileged processes,
>>>>>> but this bloating also advertises and leverages using more secure CAP_PERFMON
>>>>>> based approach to use perf_event_open system call.
>>>>>
>>>>> I can live with that.  We just need to document that when you see
>>>>> both a CAP_PERFMON and a CAP_SYS_ADMIN audit message for a process,
>>>>> try only allowing CAP_PERFMON first and see if that resolves the
>>>>> issue.  We have a similar issue with CAP_DAC_READ_SEARCH versus
>>>>> CAP_DAC_OVERRIDE.
>>>>
>>>> perf security [1] document can be updated, at least, to align and document 
>>>> this audit logging specifics.
>>>
>>> And I plan to update the document right after this patch set is accepted.
>>> Feel free to let me know of the places in the kernel docs that also
>>> require update w.r.t CAP_PERFMON extension.
>>
>> The documentation update wants be part of the patch set and not planned
>> to be done _after_ the patch set is merged.
> 
> Well, accepted. It is going to make patches #11 and beyond.

Patches #11 and #12 of v7 [1] contain information on CAP_PERFMON intention and usage.
Patch for man-pages [2] extends perf_event_open.2 documentation.

Thanks,
Alexey

---
[1] https://lore.kernel.org/lkml/c8de937a-0b3a-7147-f5ef-69f467e87a13@linux.intel.com/
[2] https://lore.kernel.org/lkml/18d1083d-efe5-f5f8-c531-d142c0e5c1a8@linux.intel.com/


^ permalink raw reply

* [PATCH v3 00/20] global exec/memory/dma APIs cleanup
From: Philippe Mathieu-Daudé @ 2020-02-20 13:05 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: Edgar E. Iglesias, Anthony Perard, Fam Zheng,
	Hervé Poussineau, Philippe Mathieu-Daudé, kvm,
	Laurent Vivier, Thomas Huth, Stefan Weil, Eric Auger, Halil Pasic,
	Marcel Apfelbaum, qemu-s390x, Aleksandar Rikalo, David Gibson,
	Michael Walle, qemu-ppc, Gerd Hoffmann, Cornelia Huck, qemu-arm,
	Alistair Francis, qemu-block, Cédric Le Goater, Jason Wang,
	xen-devel, Christian Borntraeger, Dmitry Fleytman, Matthew Rosato,
	Eduardo Habkost, Richard Henderson, Michael S. Tsirkin,
	David Hildenbrand, Paolo Bonzini, Stefano Stabellini,
	Igor Mitsyanko, Paul Durrant, Richard Henderson, John Snow

This series is inspired from Peter Maydel cleanup patch:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg680625.html

- Convert 'is_write' argument to boolean
- Use void pointer for blob buffer
- Remove unnecessary casts (Stefan Weil)
- Replace [API]_rw() by [API]_read/write() when is_write is constant

Supersedes: <20200218112457.22712-1-peter.maydell@linaro.org>

Peter Maydell (1):
  Avoid address_space_rw() with a constant is_write argument

Philippe Mathieu-Daudé (19):
  scripts/git.orderfile: Display Cocci scripts before code modifications
  hw: Remove unnecessary cast when calling dma_memory_read()
  exec: Let qemu_ram_*() functions take a const pointer argument
  exec: Rename ram_ptr variable
  exec: Let flatview API take void pointer arguments
  exec: Let the address_space API use void pointer arguments
  hw/net: Avoid casting non-const pointer, use address_space_write()
  Remove unnecessary cast when using the address_space API
  exec: Let the cpu_[physical]_memory API use void pointer arguments
  Remove unnecessary cast when using the cpu_[physical]_memory API
  hw/ide/internal: Remove unused DMARestartFunc typedef
  hw/ide: Let the DMAIntFunc prototype use a boolean 'is_write' argument
  hw/virtio: Let virtqueue_map_iovec() use a boolean 'is_write' argument
  hw/virtio: Let vhost_memory_map() use a boolean 'is_write' argument
  exec: Let address_space_unmap() use a boolean 'is_write' argument
  Let address_space_rw() calls pass a boolean 'is_write' argument
  exec: Let cpu_[physical]_memory API use a boolean 'is_write' argument
  Let cpu_[physical]_memory() calls pass a boolean 'is_write' argument
  Avoid cpu_physical_memory_rw() with a constant is_write argument

 scripts/coccinelle/exec_rw_const.cocci | 103 +++++++++++++++++++++++++
 include/exec/cpu-all.h                 |   2 +-
 include/exec/cpu-common.h              |  18 ++---
 include/exec/memory.h                  |  16 ++--
 include/hw/ide/internal.h              |   3 +-
 include/sysemu/xen-mapcache.h          |   4 +-
 target/i386/hvf/vmx.h                  |   7 +-
 accel/kvm/kvm-all.c                    |   6 +-
 dma-helpers.c                          |   4 +-
 exec.c                                 |  75 +++++++++---------
 hw/arm/boot.c                          |   6 +-
 hw/arm/smmu-common.c                   |   3 +-
 hw/arm/smmuv3.c                        |  10 +--
 hw/display/exynos4210_fimd.c           |   3 +-
 hw/display/milkymist-tmu2.c            |   8 +-
 hw/display/omap_dss.c                  |   2 +-
 hw/display/omap_lcdc.c                 |  10 +--
 hw/display/ramfb.c                     |   2 +-
 hw/dma/etraxfs_dma.c                   |  25 +++---
 hw/dma/rc4030.c                        |  10 +--
 hw/dma/xlnx-zdma.c                     |  11 +--
 hw/i386/xen/xen-mapcache.c             |   2 +-
 hw/ide/ahci.c                          |   2 +-
 hw/ide/core.c                          |   2 +-
 hw/ide/macio.c                         |   2 +-
 hw/ide/pci.c                           |   2 +-
 hw/misc/pc-testdev.c                   |   2 +-
 hw/net/cadence_gem.c                   |  21 +++--
 hw/net/dp8393x.c                       |  70 +++++++++--------
 hw/net/i82596.c                        |  25 +++---
 hw/net/lasi_i82596.c                   |   5 +-
 hw/nvram/spapr_nvram.c                 |   4 +-
 hw/ppc/pnv_lpc.c                       |   8 +-
 hw/ppc/ppc440_uc.c                     |   6 +-
 hw/ppc/spapr_hcall.c                   |   4 +-
 hw/s390x/css.c                         |  12 +--
 hw/s390x/ipl.c                         |   2 +-
 hw/s390x/s390-pci-bus.c                |   2 +-
 hw/s390x/virtio-ccw.c                  |   2 +-
 hw/scsi/vmw_pvscsi.c                   |   8 +-
 hw/sd/sdhci.c                          |  15 ++--
 hw/virtio/vhost.c                      |   8 +-
 hw/virtio/virtio.c                     |   7 +-
 hw/xen/xen_pt_graphics.c               |   2 +-
 qtest.c                                |  52 ++++++-------
 target/i386/hax-all.c                  |   6 +-
 target/i386/hvf/x86_mmu.c              |  12 +--
 target/i386/whpx-all.c                 |   2 +-
 target/s390x/excp_helper.c             |   2 +-
 target/s390x/helper.c                  |   6 +-
 target/s390x/mmu_helper.c              |   2 +-
 scripts/git.orderfile                  |   3 +
 52 files changed, 360 insertions(+), 266 deletions(-)
 create mode 100644 scripts/coccinelle/exec_rw_const.cocci

-- 
2.21.1

^ permalink raw reply

* [PATCH v3 01/20] scripts/git.orderfile: Display Cocci scripts before code modifications
From: Philippe Mathieu-Daudé @ 2020-02-20 13:05 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: Edgar E. Iglesias, Anthony Perard, Fam Zheng,
	Hervé Poussineau, Philippe Mathieu-Daudé, kvm,
	Laurent Vivier, Thomas Huth, Stefan Weil, Eric Auger, Halil Pasic,
	Marcel Apfelbaum, qemu-s390x, Aleksandar Rikalo, David Gibson,
	Michael Walle, qemu-ppc, Gerd Hoffmann, Cornelia Huck, qemu-arm,
	Alistair Francis, qemu-block, Cédric Le Goater, Jason Wang,
	xen-devel, Christian Borntraeger, Dmitry Fleytman, Matthew Rosato,
	Eduardo Habkost, Richard Henderson, Michael S. Tsirkin,
	David Hildenbrand, Paolo Bonzini, Stefano Stabellini,
	Igor Mitsyanko, Paul Durrant, Richard Henderson, John Snow
In-Reply-To: <20200220130548.29974-1-philmd@redhat.com>

When we use a Coccinelle semantic script to do automatic
code modifications, it makes sense to look at the semantic
patch first.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 scripts/git.orderfile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/git.orderfile b/scripts/git.orderfile
index 1f747b583a..7cf22e0bf5 100644
--- a/scripts/git.orderfile
+++ b/scripts/git.orderfile
@@ -22,6 +22,9 @@ Makefile*
 qapi/*.json
 qga/*.json
 
+# semantic patches
+*.cocci
+
 # headers
 *.h
 
-- 
2.21.1

^ permalink raw reply related

* [PATCH v3 02/20] hw: Remove unnecessary cast when calling dma_memory_read()
From: Philippe Mathieu-Daudé @ 2020-02-20 13:05 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: Edgar E. Iglesias, Anthony Perard, Fam Zheng,
	Hervé Poussineau, Philippe Mathieu-Daudé, kvm,
	Laurent Vivier, Thomas Huth, Stefan Weil, Eric Auger, Halil Pasic,
	Marcel Apfelbaum, qemu-s390x, Aleksandar Rikalo, David Gibson,
	Michael Walle, qemu-ppc, Gerd Hoffmann, Cornelia Huck, qemu-arm,
	Alistair Francis, qemu-block, Cédric Le Goater, Jason Wang,
	xen-devel, Christian Borntraeger, Dmitry Fleytman, Matthew Rosato,
	Eduardo Habkost, Richard Henderson, Michael S. Tsirkin,
	David Hildenbrand, Paolo Bonzini, Stefano Stabellini,
	Igor Mitsyanko, Paul Durrant, Richard Henderson, John Snow
In-Reply-To: <20200220130548.29974-1-philmd@redhat.com>

Since its introduction in commit d86a77f8abb, dma_memory_read()
always accepted void pointer argument. Remove the unnecessary
casts.

This commit was produced with the included Coccinelle script
scripts/coccinelle/exec_rw_const.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 scripts/coccinelle/exec_rw_const.cocci | 15 +++++++++++++++
 hw/arm/smmu-common.c                   |  3 +--
 hw/arm/smmuv3.c                        | 10 ++++------
 hw/sd/sdhci.c                          | 15 +++++----------
 4 files changed, 25 insertions(+), 18 deletions(-)
 create mode 100644 scripts/coccinelle/exec_rw_const.cocci

diff --git a/scripts/coccinelle/exec_rw_const.cocci b/scripts/coccinelle/exec_rw_const.cocci
new file mode 100644
index 0000000000..a0054f009d
--- /dev/null
+++ b/scripts/coccinelle/exec_rw_const.cocci
@@ -0,0 +1,15 @@
+// Usage:
+//  spatch --sp-file scripts/coccinelle/exec_rw_const.cocci --dir . --in-place
+
+// Remove useless cast
+@@
+expression E1, E2, E3, E4;
+type T;
+@@
+(
+- dma_memory_read(E1, E2, (T *)E3, E4)
++ dma_memory_read(E1, E2, E3, E4)
+|
+- dma_memory_write(E1, E2, (T *)E3, E4)
++ dma_memory_write(E1, E2, E3, E4)
+)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 23eb117041..0f2573f004 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -74,8 +74,7 @@ static int get_pte(dma_addr_t baseaddr, uint32_t index, uint64_t *pte,
     dma_addr_t addr = baseaddr + index * sizeof(*pte);
 
     /* TODO: guarantee 64-bit single-copy atomicity */
-    ret = dma_memory_read(&address_space_memory, addr,
-                          (uint8_t *)pte, sizeof(*pte));
+    ret = dma_memory_read(&address_space_memory, addr, pte, sizeof(*pte));
 
     if (ret != MEMTX_OK) {
         info->type = SMMU_PTW_ERR_WALK_EABT;
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 8b5f157dc7..57a79df55b 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -279,8 +279,7 @@ static int smmu_get_ste(SMMUv3State *s, dma_addr_t addr, STE *buf,
 
     trace_smmuv3_get_ste(addr);
     /* TODO: guarantee 64-bit single-copy atomicity */
-    ret = dma_memory_read(&address_space_memory, addr,
-                          (void *)buf, sizeof(*buf));
+    ret = dma_memory_read(&address_space_memory, addr, buf, sizeof(*buf));
     if (ret != MEMTX_OK) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "Cannot fetch pte at address=0x%"PRIx64"\n", addr);
@@ -301,8 +300,7 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint32_t ssid,
 
     trace_smmuv3_get_cd(addr);
     /* TODO: guarantee 64-bit single-copy atomicity */
-    ret = dma_memory_read(&address_space_memory, addr,
-                           (void *)buf, sizeof(*buf));
+    ret = dma_memory_read(&address_space_memory, addr, buf, sizeof(*buf));
     if (ret != MEMTX_OK) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "Cannot fetch pte at address=0x%"PRIx64"\n", addr);
@@ -406,8 +404,8 @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste,
         l2_ste_offset = sid & ((1 << s->sid_split) - 1);
         l1ptr = (dma_addr_t)(strtab_base + l1_ste_offset * sizeof(l1std));
         /* TODO: guarantee 64-bit single-copy atomicity */
-        ret = dma_memory_read(&address_space_memory, l1ptr,
-                              (uint8_t *)&l1std, sizeof(l1std));
+        ret = dma_memory_read(&address_space_memory, l1ptr, &l1std,
+                              sizeof(l1std));
         if (ret != MEMTX_OK) {
             qemu_log_mask(LOG_GUEST_ERROR,
                           "Could not read L1PTR at 0X%"PRIx64"\n", l1ptr);
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 69dc3e6b90..d5abdaad41 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -701,8 +701,7 @@ static void get_adma_description(SDHCIState *s, ADMADescr *dscr)
     hwaddr entry_addr = (hwaddr)s->admasysaddr;
     switch (SDHC_DMA_TYPE(s->hostctl1)) {
     case SDHC_CTRL_ADMA2_32:
-        dma_memory_read(s->dma_as, entry_addr, (uint8_t *)&adma2,
-                        sizeof(adma2));
+        dma_memory_read(s->dma_as, entry_addr, &adma2, sizeof(adma2));
         adma2 = le64_to_cpu(adma2);
         /* The spec does not specify endianness of descriptor table.
          * We currently assume that it is LE.
@@ -713,8 +712,7 @@ static void get_adma_description(SDHCIState *s, ADMADescr *dscr)
         dscr->incr = 8;
         break;
     case SDHC_CTRL_ADMA1_32:
-        dma_memory_read(s->dma_as, entry_addr, (uint8_t *)&adma1,
-                        sizeof(adma1));
+        dma_memory_read(s->dma_as, entry_addr, &adma1, sizeof(adma1));
         adma1 = le32_to_cpu(adma1);
         dscr->addr = (hwaddr)(adma1 & 0xFFFFF000);
         dscr->attr = (uint8_t)extract32(adma1, 0, 7);
@@ -726,13 +724,10 @@ static void get_adma_description(SDHCIState *s, ADMADescr *dscr)
         }
         break;
     case SDHC_CTRL_ADMA2_64:
-        dma_memory_read(s->dma_as, entry_addr,
-                        (uint8_t *)(&dscr->attr), 1);
-        dma_memory_read(s->dma_as, entry_addr + 2,
-                        (uint8_t *)(&dscr->length), 2);
+        dma_memory_read(s->dma_as, entry_addr, (&dscr->attr), 1);
+        dma_memory_read(s->dma_as, entry_addr + 2, (&dscr->length), 2);
         dscr->length = le16_to_cpu(dscr->length);
-        dma_memory_read(s->dma_as, entry_addr + 4,
-                        (uint8_t *)(&dscr->addr), 8);
+        dma_memory_read(s->dma_as, entry_addr + 4, (&dscr->addr), 8);
         dscr->addr = le64_to_cpu(dscr->addr);
         dscr->attr &= (uint8_t) ~0xC0;
         dscr->incr = 12;
-- 
2.21.1

^ permalink raw reply related

* [PATCH v3 03/20] exec: Let qemu_ram_*() functions take a const pointer argument
From: Philippe Mathieu-Daudé @ 2020-02-20 13:05 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: Edgar E. Iglesias, Anthony Perard, Fam Zheng,
	Hervé Poussineau, Philippe Mathieu-Daudé, kvm,
	Laurent Vivier, Thomas Huth, Stefan Weil, Eric Auger, Halil Pasic,
	Marcel Apfelbaum, qemu-s390x, Aleksandar Rikalo, David Gibson,
	Michael Walle, qemu-ppc, Gerd Hoffmann, Cornelia Huck, qemu-arm,
	Alistair Francis, qemu-block, Cédric Le Goater, Jason Wang,
	xen-devel, Christian Borntraeger, Dmitry Fleytman, Matthew Rosato,
	Eduardo Habkost, Richard Henderson, Michael S. Tsirkin,
	David Hildenbrand, Paolo Bonzini, Stefano Stabellini,
	Igor Mitsyanko, Paul Durrant, Richard Henderson, John Snow
In-Reply-To: <20200220130548.29974-1-philmd@redhat.com>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/exec/cpu-common.h     | 6 +++---
 include/sysemu/xen-mapcache.h | 4 ++--
 exec.c                        | 8 ++++----
 hw/i386/xen/xen-mapcache.c    | 2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 81753bbb34..05ac1a5d69 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -48,11 +48,11 @@ typedef uint32_t CPUReadMemoryFunc(void *opaque, hwaddr addr);
 
 void qemu_ram_remap(ram_addr_t addr, ram_addr_t length);
 /* This should not be used by devices.  */
-ram_addr_t qemu_ram_addr_from_host(void *ptr);
+ram_addr_t qemu_ram_addr_from_host(const void *ptr);
 RAMBlock *qemu_ram_block_by_name(const char *name);
-RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
+RAMBlock *qemu_ram_block_from_host(const void *ptr, bool round_offset,
                                    ram_addr_t *offset);
-ram_addr_t qemu_ram_block_host_offset(RAMBlock *rb, void *host);
+ram_addr_t qemu_ram_block_host_offset(RAMBlock *rb, const void *host);
 void qemu_ram_set_idstr(RAMBlock *block, const char *name, DeviceState *dev);
 void qemu_ram_unset_idstr(RAMBlock *block);
 const char *qemu_ram_get_idstr(RAMBlock *rb);
diff --git a/include/sysemu/xen-mapcache.h b/include/sysemu/xen-mapcache.h
index c8e7c2f6cf..81e9aa2fa6 100644
--- a/include/sysemu/xen-mapcache.h
+++ b/include/sysemu/xen-mapcache.h
@@ -19,7 +19,7 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f,
                         void *opaque);
 uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size,
                        uint8_t lock, bool dma);
-ram_addr_t xen_ram_addr_from_mapcache(void *ptr);
+ram_addr_t xen_ram_addr_from_mapcache(const void *ptr);
 void xen_invalidate_map_cache_entry(uint8_t *buffer);
 void xen_invalidate_map_cache(void);
 uint8_t *xen_replace_cache_entry(hwaddr old_phys_addr,
@@ -40,7 +40,7 @@ static inline uint8_t *xen_map_cache(hwaddr phys_addr,
     abort();
 }
 
-static inline ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
+static inline ram_addr_t xen_ram_addr_from_mapcache(const void *ptr)
 {
     abort();
 }
diff --git a/exec.c b/exec.c
index 8e9cc3b47c..02b4e6ea41 100644
--- a/exec.c
+++ b/exec.c
@@ -2614,7 +2614,7 @@ static void *qemu_ram_ptr_length(RAMBlock *ram_block, ram_addr_t addr,
 }
 
 /* Return the offset of a hostpointer within a ramblock */
-ram_addr_t qemu_ram_block_host_offset(RAMBlock *rb, void *host)
+ram_addr_t qemu_ram_block_host_offset(RAMBlock *rb, const void *host)
 {
     ram_addr_t res = (uint8_t *)host - (uint8_t *)rb->host;
     assert((uintptr_t)host >= (uintptr_t)rb->host);
@@ -2640,11 +2640,11 @@ ram_addr_t qemu_ram_block_host_offset(RAMBlock *rb, void *host)
  * pointer, such as a reference to the region that includes the incoming
  * ram_addr_t.
  */
-RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
+RAMBlock *qemu_ram_block_from_host(const void *ptr, bool round_offset,
                                    ram_addr_t *offset)
 {
     RAMBlock *block;
-    uint8_t *host = ptr;
+    const uint8_t *host = ptr;
 
     if (xen_enabled()) {
         ram_addr_t ram_addr;
@@ -2705,7 +2705,7 @@ RAMBlock *qemu_ram_block_by_name(const char *name)
 
 /* Some of the softmmu routines need to translate from a host pointer
    (typically a TLB entry) back to a ram offset.  */
-ram_addr_t qemu_ram_addr_from_host(void *ptr)
+ram_addr_t qemu_ram_addr_from_host(const void *ptr)
 {
     RAMBlock *block;
     ram_addr_t offset;
diff --git a/hw/i386/xen/xen-mapcache.c b/hw/i386/xen/xen-mapcache.c
index 5b120ed44b..432ad3354d 100644
--- a/hw/i386/xen/xen-mapcache.c
+++ b/hw/i386/xen/xen-mapcache.c
@@ -363,7 +363,7 @@ uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size,
     return p;
 }
 
-ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
+ram_addr_t xen_ram_addr_from_mapcache(const void *ptr)
 {
     MapCacheEntry *entry = NULL;
     MapCacheRev *reventry;
-- 
2.21.1

^ permalink raw reply related

* [PATCH v3 04/20] exec: Rename ram_ptr variable
From: Philippe Mathieu-Daudé @ 2020-02-20 13:05 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: Edgar E. Iglesias, Anthony Perard, Fam Zheng,
	Hervé Poussineau, Philippe Mathieu-Daudé, kvm,
	Laurent Vivier, Thomas Huth, Stefan Weil, Eric Auger, Halil Pasic,
	Marcel Apfelbaum, qemu-s390x, Aleksandar Rikalo, David Gibson,
	Michael Walle, qemu-ppc, Gerd Hoffmann, Cornelia Huck, qemu-arm,
	Alistair Francis, qemu-block, Cédric Le Goater, Jason Wang,
	xen-devel, Christian Borntraeger, Dmitry Fleytman, Matthew Rosato,
	Eduardo Habkost, Richard Henderson, Michael S. Tsirkin,
	David Hildenbrand, Paolo Bonzini, Stefano Stabellini,
	Igor Mitsyanko, Paul Durrant, Richard Henderson, John Snow
In-Reply-To: <20200220130548.29974-1-philmd@redhat.com>

As we are going to use a different 'ptr' variable, rename the 'ram
pointer' variable.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 exec.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/exec.c b/exec.c
index 02b4e6ea41..06e386dc72 100644
--- a/exec.c
+++ b/exec.c
@@ -3151,7 +3151,7 @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr,
                                            hwaddr len, hwaddr addr1,
                                            hwaddr l, MemoryRegion *mr)
 {
-    uint8_t *ptr;
+    uint8_t *ram_ptr;
     uint64_t val;
     MemTxResult result = MEMTX_OK;
     bool release_lock = false;
@@ -3167,8 +3167,8 @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr,
                                                    size_memop(l), attrs);
         } else {
             /* RAM case */
-            ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);
-            memcpy(ptr, buf, l);
+            ram_ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);
+            memcpy(ram_ptr, buf, l);
             invalidate_and_set_dirty(mr, addr1, l);
         }
 
@@ -3215,7 +3215,7 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
                                    hwaddr len, hwaddr addr1, hwaddr l,
                                    MemoryRegion *mr)
 {
-    uint8_t *ptr;
+    uint8_t *ram_ptr;
     uint64_t val;
     MemTxResult result = MEMTX_OK;
     bool release_lock = false;
@@ -3230,8 +3230,8 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
             stn_he_p(buf, l, val);
         } else {
             /* RAM case */
-            ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);
-            memcpy(buf, ptr, l);
+            ram_ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);
+            memcpy(buf, ram_ptr, l);
         }
 
         if (release_lock) {
@@ -3329,7 +3329,7 @@ static inline MemTxResult address_space_write_rom_internal(AddressSpace *as,
                                                            enum write_rom_type type)
 {
     hwaddr l;
-    uint8_t *ptr;
+    uint8_t *ram_ptr;
     hwaddr addr1;
     MemoryRegion *mr;
 
@@ -3343,14 +3343,14 @@ static inline MemTxResult address_space_write_rom_internal(AddressSpace *as,
             l = memory_access_size(mr, l, addr1);
         } else {
             /* ROM/RAM case */
-            ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
+            ram_ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
             switch (type) {
             case WRITE_DATA:
-                memcpy(ptr, buf, l);
+                memcpy(ram_ptr, buf, l);
                 invalidate_and_set_dirty(mr, addr1, l);
                 break;
             case FLUSH_CACHE:
-                flush_icache_range((uintptr_t)ptr, (uintptr_t)ptr + l);
+                flush_icache_range((uintptr_t)ram_ptr, (uintptr_t)ram_ptr + l);
                 break;
             }
         }
-- 
2.21.1

^ permalink raw reply related

* [PATCH v4 15/16] s390x: Add unpack feature to GA1
From: Janosch Frank @ 2020-02-20 12:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-s390x, mihajlov, david, cohuck
In-Reply-To: <20200220125638.7241-1-frankja@linux.ibm.com>

From: Christian Borntraeger <borntraeger@de.ibm.com>

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target/s390x/gen-features.c | 1 +
 target/s390x/kvm.c          | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 6278845b12..8ddeebc544 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -562,6 +562,7 @@ static uint16_t full_GEN15_GA1[] = {
     S390_FEAT_GROUP_MSA_EXT_9,
     S390_FEAT_GROUP_MSA_EXT_9_PCKMO,
     S390_FEAT_ETOKEN,
+    S390_FEAT_UNPACK,
 };
 
 /* Default features (in order of release)
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 31dd49729b..5b6a7ca466 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -154,6 +154,7 @@ static int cap_ri;
 static int cap_gs;
 static int cap_hpage_1m;
 static int cap_vcpu_resets;
+static int cap_protected;
 
 static int active_cmma;
 
@@ -346,6 +347,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
     cap_mem_op = kvm_check_extension(s, KVM_CAP_S390_MEM_OP);
     cap_s390_irq = kvm_check_extension(s, KVM_CAP_S390_INJECT_IRQ);
     cap_vcpu_resets = kvm_check_extension(s, KVM_CAP_S390_VCPU_RESETS);
+    cap_protected = kvm_check_extension(s, KVM_CAP_S390_PROTECTED);
 
     if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
         || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
@@ -2394,6 +2396,11 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
         clear_bit(S390_FEAT_BPB, model->features);
     }
 
+    /* we do have the IPL enhancements */
+    if (cap_protected) {
+        set_bit(S390_FEAT_UNPACK, model->features);
+    }
+
     /* We emulate a zPCI bus and AEN, therefore we don't need HW support */
     set_bit(S390_FEAT_ZPCI, model->features);
     set_bit(S390_FEAT_ADAPTER_EVENT_NOTIFICATION, model->features);
-- 
2.20.1



^ permalink raw reply related


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.