* [Bug 98170] [vdpau] Error when calling vdp_output_surface_create
From: bugzilla-daemon @ 2016-11-14 16:22 UTC (permalink / raw)
To: dri-devel
In-Reply-To: <bug-98170-502@http.bugs.freedesktop.org/>
[-- Attachment #1.1: Type: text/plain, Size: 207 bytes --]
https://bugs.freedesktop.org/show_bug.cgi?id=98170
--- Comment #8 from Branko <bagzy92@gmail.com> ---
FIXED In mesa-13.0.1.
--
You are receiving this mail because:
You are the assignee for the bug.
[-- Attachment #1.2: Type: text/html, Size: 1038 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [Bug 98170] [vdpau] Error when calling vdp_output_surface_create
From: bugzilla-daemon @ 2016-11-14 16:21 UTC (permalink / raw)
To: dri-devel
In-Reply-To: <bug-98170-502@http.bugs.freedesktop.org/>
[-- Attachment #1.1: Type: text/plain, Size: 420 bytes --]
https://bugs.freedesktop.org/show_bug.cgi?id=98170
Branko <bagzy92@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--
You are receiving this mail because:
You are the assignee for the bug.
[-- Attachment #1.2: Type: text/html, Size: 1241 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 1/6] perf config: Add support for getting config key-value pairs
From: Taeung Song @ 2016-11-14 16:21 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
Peter Zijlstra, Wang Nan, Nambong Ha, Wookje Kwon
In-Reply-To: <20161114155050.GB26543@kernel.org>
Hi, Arnaldo
Thank you for your reply :)
I was worried lest you don't it..
On 11/15/2016 12:50 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 04, 2016 at 03:44:17PM +0900, Taeung Song escreveu:
>> Add a functionality getting specific config key-value pairs.
>> For the syntax examples,
>>
>> perf config [<file-option>] [section.name ...]
>>
>> e.g. To query config items 'report.queue-size' and 'report.children', do
>>
>> # perf config report.queue-size report.children
>
> So, I'm applying it, but while testing I noticed that it shows only the
> options that were explicitely set:
>
> [acme@jouet linux]$ perf config report.queue-size report.children
> report.children=false
> [acme@jouet linux]$
>
> Perhaps we should, in a follow up patch, show this instead:
>
> [acme@jouet linux]$ perf config report.queue-size report.children
> report.children=false
> # report.queue-size=18446744073709551615 # Default, not set in ~/.perfconfig
> [acme@jouet linux]$
>
> ?
Yes, I agree it.
I also think we should get not only config info in config files
but also inbuilt default config info.
After this patchset (support config read/wirte) is applied,
I'll send a follow up patch that contains default config array
to show inbuilt default config info as you said!!
Is it OK ?
Thanks,
Taeung
>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Cc: Wang Nan <wangnan0@huawei.com>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>> ---
>> tools/perf/builtin-config.c | 40 +++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 37 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
>> index e4207a2..df3fa1c 100644
>> --- a/tools/perf/builtin-config.c
>> +++ b/tools/perf/builtin-config.c
>> @@ -17,7 +17,7 @@
>> static bool use_system_config, use_user_config;
>>
>> static const char * const config_usage[] = {
>> - "perf config [<file-option>] [options]",
>> + "perf config [<file-option>] [options] [section.name ...]",
>> NULL
>> };
>>
>> @@ -33,6 +33,36 @@ static struct option config_options[] = {
>> OPT_END()
>> };
>>
>> +static int show_spec_config(struct perf_config_set *set, const char *var)
>> +{
>> + struct perf_config_section *section;
>> + struct perf_config_item *item;
>> +
>> + if (set == NULL)
>> + return -1;
>> +
>> + perf_config_items__for_each_entry(&set->sections, section) {
>> + if (prefixcmp(var, section->name) != 0)
>> + continue;
>> +
>> + perf_config_items__for_each_entry(§ion->items, item) {
>> + const char *name = var + strlen(section->name) + 1;
>> +
>> + if (strcmp(name, item->name) == 0) {
>> + char *value = item->value;
>> +
>> + if (value) {
>> + printf("%s=%s\n", var, value);
>> + return 0;
>> + }
>> + }
>> +
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int show_config(struct perf_config_set *set)
>> {
>> struct perf_config_section *section;
>> @@ -54,7 +84,7 @@ static int show_config(struct perf_config_set *set)
>>
>> int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>> {
>> - int ret = 0;
>> + int i, ret = 0;
>> struct perf_config_set *set;
>> char *user_config = mkpath("%s/.perfconfig", getenv("HOME"));
>>
>> @@ -100,7 +130,11 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>> }
>> break;
>> default:
>> - usage_with_options(config_usage, config_options);
>> + if (argc)
>> + for (i = 0; argv[i]; i++)
>> + ret = show_spec_config(set, argv[i]);
>> + else
>> + usage_with_options(config_usage, config_options);
>> }
>>
>> perf_config_set__delete(set);
>> --
>> 2.7.4
^ permalink raw reply
* Re: [RFC v2 8/8] iommu/arm-smmu: implement add_reserved_regions callback
From: Joerg Roedel @ 2016-11-14 16:20 UTC (permalink / raw)
To: Auger Eric
Cc: drjones, alex.williamson, jason, kvm, marc.zyngier, punit.agrawal,
will.deacon, linux-kernel, diana.craciun, iommu,
pranav.sawargaonkar, christoffer.dall, tglx, robin.murphy,
linux-arm-kernel, eric.auger.pro
In-Reply-To: <295feefe-014b-5669-7f5a-e04b09ba3454@redhat.com>
On Mon, Nov 14, 2016 at 05:08:16PM +0100, Auger Eric wrote:
> There are potentially several MSI doorbell physical pages in the SOC
> that are accessed through the IOMMU (translated). Each of those must
> have a corresponding IOVA and IOVA/PA mapping programmed in the IOMMU.
> Else MSI will fault.
>
> - step 1 was to define a usable IOVA range for MSI mapping. So now we
> decided the base address and size would be hardcoded for ARM. The
> get_dm_region can be used to retrieve that hardcoded region.
> - Step2 is to allocate IOVAs within that range and map then for each of
> those MSI doorbells. This is done in the MSI controller compose() callback.
>
> I hope I succeeded in clarifying this time.
>
> Robin sent today a new version of its cookie think using a dummy
> allocator. I am currently integrating it.
Okay, I understand. A simple bitmap-allocator would probably be
sufficient, and doesn't have the overhead the iova allocator has. About
how many pages are we talking here?
Joerg
^ permalink raw reply
* Re: [PATCH 1/2] Revert "drm: Add and handle new aspect ratios in DRM layer"
From: Ville Syrjälä @ 2016-11-14 16:20 UTC (permalink / raw)
To: Sharma, Shashank
Cc: Jose Abreu, Jia, Lin A, Akashdeep Sharma, Emil Velikov, dri-devel,
Daniel Vetter, Jim Bride
In-Reply-To: <d981a93b-e46f-787b-3565-3b20b0628649@intel.com>
On Mon, Nov 14, 2016 at 09:37:18PM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 11/14/2016 9:19 PM, Ville Syrjälä wrote:
> > On Mon, Nov 14, 2016 at 08:14:34PM +0530, Sharma, Shashank wrote:
> >> Regards
> >> Shashank
> >>> the revert:
> >>>
> >>> HDMI2 connected 1920x1080+0+0 (normal left inverted right x axis y axis) 700mm x 390mm
> >>> - 1920x1080 60.00*+
> >>> - 1920x1080i 60.00 50.00
> >>> + 1920x1080 60.00*+ 50.00 59.94 30.00 25.00 24.00 29.97 23.98
> >>> + 1920x1080i 60.00 50.00 59.94
> >>> 1600x1200 60.00
> >>> 1680x1050 59.88
> >>> 1280x1024 75.02 60.02
> >>> @@ -13,30 +13,29 @@
> >>> 1360x768 60.02
> >>> 1280x800 59.91
> >>> 1152x864 75.00
> >>> - 1280x720 60.00 50.00
> >>> + 1280x720 60.00 50.00 59.94
> >>> 1024x768 75.03 70.07 60.00
> >>> 832x624 74.55
> >>> 800x600 72.19 75.00 60.32
> >>> - 640x480 75.00 72.81 66.67 59.94
> >>> + 720x576 50.00
> >>> + 720x480 60.00 59.94
> >>> + 640x480 75.00 72.81 66.67 60.00 59.94
> >>> 720x400 70.08
> >> None of these aspect ratios are new modes / new aspect ratios from HDMI
> >> 2.0/CEA-861-F
> >> These are the existing modes, and should be independent of reverted
> >> patches.
> > They're affected because your patches changed them by adding the aspect
> > ratio flags to them.
> Yes, But they are independent of reverted patch, which adds aspect ratio
> for HDMI 2.0 ratios (64:27 and 256:135)
The second patch had to be reverted so that the first patch would revert
cleanly.
> >>> This was with sna, which does this:
> >>> #define KNOWN_MODE_FLAGS ((1<<14)-1)
> >>> if (mode->status == MODE_OK && kmode->flags & ~KNOWN_MODE_FLAGS)
> >>> mode->status = MODE_BAD; /* unknown flags => unhandled */
> >>> so all the modes with an aspect ratio just vanished.
> >>>
> >>> -modesetting and -ati on the other hand just copy over the unknown
> >>> bits into the xrandr mode structure, which sounds dubious at best:
> >>> mode->Flags = kmode->flags; //& FLAG_BITS;
> >>> I've not checked what damage it can actually cause.
> >>>
> >>>
> >>> It looks like a few modes disappeared from the kernel's mode list
> >>> as well, presumably because some cea modes in the list originated from
> >>> DTDs and whanot so they don't have an aspect ratio and that causes
> >>> add_alternate_cea_modes() to ignore them. So not populating an aspect
> >>> ratio for cea modes originating from a source other than
> >>> edid_cea_modes[] looks like another bug to me as well.
> >> I am writing a patch series to cap the aspect ratio implementation under
> >> a drm_cap_hdmi2_aspect_ratios
> >> This is how its going to work (inspired from the 2D/stereo series from
> >> damien L)
> >>
> >> - Add a new capability hdmi2_ar
> > It should be just a generic "expose aspect ratio flags to userspace?"
> Makes sense, in this way we can even revert the aspect_ratio property
> for HDMI connector, as discussed during
> the code review sessions of this patch series. In this way, when kernel
> will expose the aspect ratios, it will either
> do the aspect ratios as per EDID, or wont.
> >
> >> - by default parsing the new hdmi 2.0 aspect ratio will be disabled
> >> under check of this cap
> >> - during bootup time, while initializing the display, a userspace can
> >> get_cap on the hdmi2_aspect_ratio
> >> - If it wants HDMI 2.0 aspect ratio support, it will set the cap, and
> >> kernel will expose these aspect ratios
> >>> Another bug I think might be the ordering of the modes with aspect ratio
> >>> specified. IIRC the spec says that the preferred aspect ratio should be
> >>> listed first in the EDID, but I don't think we preserve that ordering
> >>> in the final mode list. I guess we could fix that by somehow noting
> >>> which aspect ratio is preferred and sort based on that, or we try to
> >>> preserve the order from the EDID until we're ready to sort, and then do
> >>> the sorting with a stable algorithm.
> >> AFAIK The mode order and priority is decided and arranged in userspace,
> >> based on various factors like
> >> - preferred mode.
> >> - previously applied mode in previous sessions (like for android tvs)
> >> - Bigger h/w vs better refresh rate ?
> >> - Xserver applies its own algorithms to decide which mode should be
> >> shown first.
> > Xorg does sort on its own. But since it doesn't know anything about
> > aspect ratios and whatnot I wouldn't rely on that for anything. I
> > also wouldn't expect eg. wayland compositors to do their own sorting.
> > And yeah, looks like weston at least doesn't do any sorting whatsoever.
> >
> >> I dont think kernel needs to bother about it.
> > So I'm going to say that we in fact do need to bother.
> >
> IMHO, making policies for UI is not a part of kernel design, a UI
> manager (Hardware composed, X or Wayland) should take care of it, as
> they have access to much information (Like previously applied mode, user
> preference etc). When it comes to sorting of modes, the only general rule
> across drivers like FB, V4L2, I have seen is the first mode in the list
> should be preferred mode, which we are still keeping. And after that our
> probed_modes were
> anyways not sorted now, so it doesn't matter further.
Having userspace be responsible for sorting the aspect ratios would
perhaps require that userspace parses the EDID, which is pretty crazy.
I guess it could try to deduce something from the physical aspect ratio
of the display, but I'm not sure that's quite what we want either.
Also we already sort the modes in the kernel anyway, so it's not like
we'd be doing something new by also considering the aspect ratios.
I would at the very least want to avoid a totally random order between
modes that differ only by the aspect ratio.
>
> If X server doesn't know what to do with aspect ratio flags, it can
> chose not to set the cap, and if HWC knows, it can chose to set. This is
> the same situation as 2D stereo modes
> which are existing already.
>
> Regards
> Shashank
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH] lib/oe/package_manager: .deb pre/postinst args
From: Linus Wallgren @ 2016-11-14 16:20 UTC (permalink / raw)
To: openembedded-core; +Cc: Linus Wallgren
In-Reply-To: <CAJTo0LZQxKxVVTPY3FEwRNxsF_V18E9Uf9Qm0RiBNWnu67G2VQ@mail.gmail.com>
The debian policy manual and MaintainerScripts wiki page states that the
postinst script is supposed to be called with the `configure` argument
at first install, likewise the preinst script is supposed to be called
with the `install` argument on first install.
https://www.debian.org/doc/debian-policy/ch-maintainerscripts.html
https://wiki.debian.org/MaintainerScripts
Signed-off-by: Linus Wallgren <linus.wallgren@scypho.com>
---
meta/lib/oe/package_manager.py | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/meta/lib/oe/package_manager.py b/meta/lib/oe/package_manager.py
index 3cee973..ec947c3 100644
--- a/meta/lib/oe/package_manager.py
+++ b/meta/lib/oe/package_manager.py
@@ -1993,7 +1993,10 @@ class DpkgPM(OpkgDpkgPM):
"""
def run_pre_post_installs(self, package_name=None):
info_dir = self.target_rootfs + "/var/lib/dpkg/info"
- suffixes = [(".preinst", "Preinstall"), (".postinst", "Postinstall")]
+ ControlScript = collections.namedtuple("ControlScript", ["suffix", "name", "argument"])
+ control_scripts = [
+ ControlScript(".preinst", "Preinstall", "install"),
+ ControlScript(".postinst", "Postinstall", "configure")]
status_file = self.target_rootfs + "/var/lib/dpkg/status"
installed_pkgs = []
@@ -2016,16 +2019,18 @@ class DpkgPM(OpkgDpkgPM):
failed_pkgs = []
for pkg_name in installed_pkgs:
- for suffix in suffixes:
- p_full = os.path.join(info_dir, pkg_name + suffix[0])
+ for control_script in control_scripts:
+ p_full = os.path.join(info_dir, pkg_name + control_script.suffix)
if os.path.exists(p_full):
try:
bb.note("Executing %s for package: %s ..." %
- (suffix[1].lower(), pkg_name))
- subprocess.check_output(p_full, stderr=subprocess.STDOUT)
+ (control_script.name.lower(), pkg_name))
+ subprocess.check_output([p_full, control_script.argument],
+ stderr=subprocess.STDOUT)
except subprocess.CalledProcessError as e:
bb.note("%s for package %s failed with %d:\n%s" %
- (suffix[1], pkg_name, e.returncode, e.output.decode("utf-8")))
+ (control_script.name, pkg_name, e.returncode,
+ e.output.decode("utf-8")))
failed_pkgs.append(pkg_name)
break
--
2.10.2
^ permalink raw reply related
* [RFC v2 8/8] iommu/arm-smmu: implement add_reserved_regions callback
From: Joerg Roedel @ 2016-11-14 16:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <295feefe-014b-5669-7f5a-e04b09ba3454@redhat.com>
On Mon, Nov 14, 2016 at 05:08:16PM +0100, Auger Eric wrote:
> There are potentially several MSI doorbell physical pages in the SOC
> that are accessed through the IOMMU (translated). Each of those must
> have a corresponding IOVA and IOVA/PA mapping programmed in the IOMMU.
> Else MSI will fault.
>
> - step 1 was to define a usable IOVA range for MSI mapping. So now we
> decided the base address and size would be hardcoded for ARM. The
> get_dm_region can be used to retrieve that hardcoded region.
> - Step2 is to allocate IOVAs within that range and map then for each of
> those MSI doorbells. This is done in the MSI controller compose() callback.
>
> I hope I succeeded in clarifying this time.
>
> Robin sent today a new version of its cookie think using a dummy
> allocator. I am currently integrating it.
Okay, I understand. A simple bitmap-allocator would probably be
sufficient, and doesn't have the overhead the iova allocator has. About
how many pages are we talking here?
Joerg
^ permalink raw reply
* Re: [RFC v2 8/8] iommu/arm-smmu: implement add_reserved_regions callback
From: Joerg Roedel @ 2016-11-14 16:20 UTC (permalink / raw)
To: Auger Eric
Cc: drjones-H+wXaHxf7aLQT0dZR+AlfA, jason-NLaQJdtUoK4Be96aLqz0jA,
kvm-u79uwXL29TY76Z2rM5mHXA, marc.zyngier-5wv7dgnIgG8,
punit.agrawal-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
pranav.sawargaonkar-Re5JQEeQqe8AvxtiuMwx3w,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
tglx-hfZtesqFncYOwBW4kG4KsQ,
christoffer.dall-QSEj5FYQhm4dnm+yROfE0A,
eric.auger.pro-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <295feefe-014b-5669-7f5a-e04b09ba3454-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Mon, Nov 14, 2016 at 05:08:16PM +0100, Auger Eric wrote:
> There are potentially several MSI doorbell physical pages in the SOC
> that are accessed through the IOMMU (translated). Each of those must
> have a corresponding IOVA and IOVA/PA mapping programmed in the IOMMU.
> Else MSI will fault.
>
> - step 1 was to define a usable IOVA range for MSI mapping. So now we
> decided the base address and size would be hardcoded for ARM. The
> get_dm_region can be used to retrieve that hardcoded region.
> - Step2 is to allocate IOVAs within that range and map then for each of
> those MSI doorbells. This is done in the MSI controller compose() callback.
>
> I hope I succeeded in clarifying this time.
>
> Robin sent today a new version of its cookie think using a dummy
> allocator. I am currently integrating it.
Okay, I understand. A simple bitmap-allocator would probably be
sufficient, and doesn't have the overhead the iova allocator has. About
how many pages are we talking here?
Joerg
^ permalink raw reply
* Re: [PATCH 03/16] ARM: berlin: use generic API for enabling SCU
From: Pankaj Dubey @ 2016-11-14 16:20 UTC (permalink / raw)
To: Jisheng Zhang
Cc: geert+renesas, Arnd Bergmann, vireshk, Magnus Damm, linux-kernel,
Krzysztof Kozlowski, rmk+kernel, Simon Horman,
thomas.ab@samsung.com, Shiraz Hashim,
linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth
In-Reply-To: <20161114165134.411ae04b@xhacker>
Hi Jisheng,
On 14 November 2016 at 14:21, Jisheng Zhang <jszhang@marvell.com> wrote:
> Hi Pankaj,
>
> On Mon, 14 Nov 2016 10:31:58 +0530 Pankaj Dubey wrote:
>
>> Now as we have of_scu_enable which takes care of mapping
>> scu base from DT, lets use it.
>>
>> CC: Jisheng Zhang <jszhang@marvell.com>
>> CC: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-berlin/platsmp.c | 17 +++++------------
>> 1 file changed, 5 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/mach-berlin/platsmp.c b/arch/arm/mach-berlin/platsmp.c
>> index 93f9068..25a6ca5 100644
>> --- a/arch/arm/mach-berlin/platsmp.c
>> +++ b/arch/arm/mach-berlin/platsmp.c
>> @@ -60,26 +60,21 @@ static int berlin_boot_secondary(unsigned int cpu, struct task_struct *idle)
>> static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
>> {
>> struct device_node *np;
>> - void __iomem *scu_base;
>> void __iomem *vectors_base;
>>
>> - np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
>> - scu_base = of_iomap(np, 0);
>> - of_node_put(np);
>> - if (!scu_base)
>> - return;
>> -
>> np = of_find_compatible_node(NULL, NULL, "marvell,berlin-cpu-ctrl");
>> cpu_ctrl = of_iomap(np, 0);
>> of_node_put(np);
>> if (!cpu_ctrl)
>> - goto unmap_scu;
>> + return;
>>
>> vectors_base = ioremap(CONFIG_VECTORS_BASE, SZ_32K);
>> if (!vectors_base)
>> - goto unmap_scu;
>> + return;
>> +
>> + if (of_scu_enable())
>
> In err code path, we need to unmap vectors_base before return
>
You are correct. I missed this, Will update in v2.
Thanks for review.
Pankaj Dubey
>> + return;
>>
>> - scu_enable(scu_base);
>> flush_cache_all();
>>
>> /*
>> @@ -95,8 +90,6 @@ static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
>> writel(virt_to_phys(secondary_startup), vectors_base + SW_RESET_ADDR);
>>
>> iounmap(vectors_base);
>> -unmap_scu:
>> - iounmap(scu_base);
>> }
>>
>> #ifdef CONFIG_HOTPLUG_CPU
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH 3/5] media: Add new SDR formats SC16, SC18 & SC20
From: Ramesh Shanmugasundaram @ 2016-11-14 16:20 UTC (permalink / raw)
To: Hans Verkuil, robh+dt@kernel.org, mark.rutland@arm.com,
mchehab@kernel.org, sakari.ailus@linux.intel.com, crope@iki.fi
Cc: Chris Paterson, laurent.pinchart@ideasonboard.com,
geert+renesas@glider.be, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <502a606c-2d66-4257-af17-7b7f35f2c839@xs4all.nl>
Hi Hans,
Thanks for the review comments.
> Subject: Re: [PATCH 3/5] media: Add new SDR formats SC16, SC18 & SC20
>
> On 11/09/2016 04:44 PM, Ramesh Shanmugasundaram wrote:
> > This patch adds support for the three new SDR formats. These formats
> > were prefixed with "sliced" indicating I data constitutes the top half
> > and Q data constitutes the bottom half of the received buffer.
>
> The standard terminology for video formats is "planar". I am leaning
> towards using that here as well.
>
> Any opinions on this?
Shall I rename the formats as "PC16", "PC18" & "PC20"?
For formats that do IQ IQ IQ... I shall use the regular formats "CUXX" when I introduce them.
Thanks,
Ramesh
>
> Hans
>
> >
> > V4L2_SDR_FMT_SCU16BE - 14-bit complex (I & Q) unsigned big-endian
> > sample inside 16-bit. V4L2 FourCC: SC16
> >
> > V4L2_SDR_FMT_SCU18BE - 16-bit complex (I & Q) unsigned big-endian
> > sample inside 18-bit. V4L2 FourCC: SC18
> >
> > V4L2_SDR_FMT_SCU20BE - 18-bit complex (I & Q) unsigned big-endian
> > sample inside 20-bit. V4L2 FourCC: SC20
> >
> > Signed-off-by: Ramesh Shanmugasundaram
> > <ramesh.shanmugasundaram@bp.renesas.com>
> > ---
> > drivers/media/v4l2-core/v4l2-ioctl.c | 3 +++
> > include/uapi/linux/videodev2.h | 3 +++
> > 2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> > b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index 181381d..d36b386 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1207,6 +1207,9 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc
> *fmt)
> > case V4L2_SDR_FMT_CS8: descr = "Complex S8"; break;
> > case V4L2_SDR_FMT_CS14LE: descr = "Complex S14LE"; break;
> > case V4L2_SDR_FMT_RU12LE: descr = "Real U12LE"; break;
> > + case V4L2_SDR_FMT_SCU16BE: descr = "Sliced Complex U16BE"; break;
> > + case V4L2_SDR_FMT_SCU18BE: descr = "Sliced Complex U18BE"; break;
> > + case V4L2_SDR_FMT_SCU20BE: descr = "Sliced Complex U20BE"; break;
> > case V4L2_TCH_FMT_DELTA_TD16: descr = "16-bit signed deltas"; break;
> > case V4L2_TCH_FMT_DELTA_TD08: descr = "8-bit signed deltas"; break;
> > case V4L2_TCH_FMT_TU16: descr = "16-bit unsigned touch data";
> break;
> > diff --git a/include/uapi/linux/videodev2.h
> > b/include/uapi/linux/videodev2.h index 4364ce6..34a9c30 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -666,6 +666,9 @@ struct v4l2_pix_format {
> > #define V4L2_SDR_FMT_CS8 v4l2_fourcc('C', 'S', '0', '8') /*
> complex s8 */
> > #define V4L2_SDR_FMT_CS14LE v4l2_fourcc('C', 'S', '1', '4') /*
> complex s14le */
> > #define V4L2_SDR_FMT_RU12LE v4l2_fourcc('R', 'U', '1', '2') /*
> real u12le */
> > +#define V4L2_SDR_FMT_SCU16BE v4l2_fourcc('S', 'C', '1', '6') /*
> sliced complex u16be */
> > +#define V4L2_SDR_FMT_SCU18BE v4l2_fourcc('S', 'C', '1', '8') /*
> sliced complex u18be */
> > +#define V4L2_SDR_FMT_SCU20BE v4l2_fourcc('S', 'C', '2', '0') /*
> sliced complex u20be */
> >
> > /* Touch formats - used for Touch devices */
> > #define V4L2_TCH_FMT_DELTA_TD16 v4l2_fourcc('T', 'D', '1', '6') /* 16-
> bit signed deltas */
> >
^ permalink raw reply
* Re: [PATCH v5] media: et8ek8: add device tree binding documentation
From: Rob Herring @ 2016-11-14 16:20 UTC (permalink / raw)
To: Pavel Machek
Cc: ivo.g.dimitrov.75, sakari.ailus, sre, pali.rohar, linux-media,
pawel.moll, mark.rutland, ijc+devicetree, galak, mchehab,
devicetree, linux-kernel
In-Reply-To: <20161107104648.GB5326@amd>
On Mon, Nov 07, 2016 at 11:46:48AM +0100, Pavel Machek wrote:
> Add device tree binding documentation for toshiba et8ek8 sensor.
>
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt b/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
> new file mode 100644
> index 0000000..b03b21d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
> @@ -0,0 +1,53 @@
> +Toshiba et8ek8 5MP sensor
> +
> +Toshiba et8ek8 5MP sensor is an image sensor found in Nokia N900 device
> +
> +More detailed documentation can be found in
> +Documentation/devicetree/bindings/media/video-interfaces.txt .
> +
> +
> +Mandatory properties
> +--------------------
> +
> +- compatible: "toshiba,et8ek8"
> +- reg: I2C address (0x3e, or an alternative address)
> +- vana-supply: Analogue voltage supply (VANA), 2.8 volts
> +- clocks: External clock to the sensor
> +- clock-frequency: Frequency of the external clock to the sensor. Camera
> + driver will set this frequency on the external clock. The clock frequency is
> + a pre-determined frequency known to be suitable to the board.
> +- reset-gpios: XSHUTDOWN GPIO. The XSHUTDOWN signal is active high. The sensor
> + is in hardware standby mode when the signal is in low state.
Sounds like active low to me. "Active" means when is the defined
function of the pin enabled, not when is the chip active/enabled. So in
this case reset or shutdown is active when low.
Rob
^ permalink raw reply
* [PATCH 2/2] NFSv4: Don't call close if the open stateid has already been cleared
From: Trond Myklebust @ 2016-11-14 16:19 UTC (permalink / raw)
To: linux-nfs; +Cc: Benjamin Coddington, Jeff Layton
In-Reply-To: <1479140396-17779-2-git-send-email-trond.myklebust@primarydata.com>
Ensure we test to see if the open stateid is actually set, before we
send a CLOSE.
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
fs/nfs/nfs4proc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index b7b0080977c0..b801040c9585 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3129,7 +3129,8 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
} else if (is_rdwr)
calldata->arg.fmode |= FMODE_READ|FMODE_WRITE;
- if (!nfs4_valid_open_stateid(state))
+ if (!nfs4_valid_open_stateid(state) ||
+ test_bit(NFS_OPEN_STATE, &state->flags) == 0)
call_close = 0;
spin_unlock(&state->owner->so_lock);
--
2.7.4
^ permalink raw reply related
* [PATCH 03/16] ARM: berlin: use generic API for enabling SCU
From: Pankaj Dubey @ 2016-11-14 16:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161114165134.411ae04b@xhacker>
Hi Jisheng,
On 14 November 2016 at 14:21, Jisheng Zhang <jszhang@marvell.com> wrote:
> Hi Pankaj,
>
> On Mon, 14 Nov 2016 10:31:58 +0530 Pankaj Dubey wrote:
>
>> Now as we have of_scu_enable which takes care of mapping
>> scu base from DT, lets use it.
>>
>> CC: Jisheng Zhang <jszhang@marvell.com>
>> CC: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-berlin/platsmp.c | 17 +++++------------
>> 1 file changed, 5 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/mach-berlin/platsmp.c b/arch/arm/mach-berlin/platsmp.c
>> index 93f9068..25a6ca5 100644
>> --- a/arch/arm/mach-berlin/platsmp.c
>> +++ b/arch/arm/mach-berlin/platsmp.c
>> @@ -60,26 +60,21 @@ static int berlin_boot_secondary(unsigned int cpu, struct task_struct *idle)
>> static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
>> {
>> struct device_node *np;
>> - void __iomem *scu_base;
>> void __iomem *vectors_base;
>>
>> - np = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-scu");
>> - scu_base = of_iomap(np, 0);
>> - of_node_put(np);
>> - if (!scu_base)
>> - return;
>> -
>> np = of_find_compatible_node(NULL, NULL, "marvell,berlin-cpu-ctrl");
>> cpu_ctrl = of_iomap(np, 0);
>> of_node_put(np);
>> if (!cpu_ctrl)
>> - goto unmap_scu;
>> + return;
>>
>> vectors_base = ioremap(CONFIG_VECTORS_BASE, SZ_32K);
>> if (!vectors_base)
>> - goto unmap_scu;
>> + return;
>> +
>> + if (of_scu_enable())
>
> In err code path, we need to unmap vectors_base before return
>
You are correct. I missed this, Will update in v2.
Thanks for review.
Pankaj Dubey
>> + return;
>>
>> - scu_enable(scu_base);
>> flush_cache_all();
>>
>> /*
>> @@ -95,8 +90,6 @@ static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
>> writel(virt_to_phys(secondary_startup), vectors_base + SW_RESET_ADDR);
>>
>> iounmap(vectors_base);
>> -unmap_scu:
>> - iounmap(scu_base);
>> }
>>
>> #ifdef CONFIG_HOTPLUG_CPU
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 1/2] NFSv4: Fix CLOSE races with OPEN
From: Trond Myklebust @ 2016-11-14 16:19 UTC (permalink / raw)
To: linux-nfs; +Cc: Benjamin Coddington, Jeff Layton
In-Reply-To: <1479140396-17779-1-git-send-email-trond.myklebust@primarydata.com>
If the reply to a successful CLOSE call races with an OPEN to the same
file, we can end up scribbling over the stateid that represents the
new open state.
The race looks like:
Client Server
====== ======
CLOSE stateid A on file "foo"
CLOSE stateid A, return stateid C
OPEN file "foo"
OPEN "foo", return stateid B
Receive reply to OPEN
Reset open state for "foo"
Associate stateid B to "foo"
Receive CLOSE for A
Reset open state for "foo"
Replace stateid B with C
The fix is to examine the argument of the CLOSE, and check for a match
with the current stateid "other" field. If the two do not match, then
the above race occurred, and we should just ignore the CLOSE.
Reported-by: Benjamin Coddington <bcodding@redhat.com>
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
fs/nfs/nfs4_fs.h | 7 +++++++
fs/nfs/nfs4proc.c | 12 ++++++------
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 9b3a82abab07..1452177c822d 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -542,6 +542,13 @@ static inline bool nfs4_valid_open_stateid(const struct nfs4_state *state)
return test_bit(NFS_STATE_RECOVERY_FAILED, &state->flags) == 0;
}
+static inline bool nfs4_state_match_open_stateid_other(const struct nfs4_state *state,
+ const nfs4_stateid *stateid)
+{
+ return test_bit(NFS_OPEN_STATE, &state->flags) &&
+ nfs4_stateid_match_other(&state->open_stateid, stateid);
+}
+
#else
#define nfs4_close_state(a, b) do { } while (0)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index f550ac69ffa0..b7b0080977c0 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1458,7 +1458,6 @@ static void nfs_resync_open_stateid_locked(struct nfs4_state *state)
}
static void nfs_clear_open_stateid_locked(struct nfs4_state *state,
- nfs4_stateid *arg_stateid,
nfs4_stateid *stateid, fmode_t fmode)
{
clear_bit(NFS_O_RDWR_STATE, &state->flags);
@@ -1476,10 +1475,9 @@ static void nfs_clear_open_stateid_locked(struct nfs4_state *state,
}
if (stateid == NULL)
return;
- /* Handle races with OPEN */
- if (!nfs4_stateid_match_other(arg_stateid, &state->open_stateid) ||
- (nfs4_stateid_match_other(stateid, &state->open_stateid) &&
- !nfs4_stateid_is_newer(stateid, &state->open_stateid))) {
+ /* Handle OPEN+OPEN_DOWNGRADE races */
+ if (nfs4_stateid_match_other(stateid, &state->open_stateid) &&
+ !nfs4_stateid_is_newer(stateid, &state->open_stateid)) {
nfs_resync_open_stateid_locked(state);
return;
}
@@ -1493,7 +1491,9 @@ static void nfs_clear_open_stateid(struct nfs4_state *state,
nfs4_stateid *stateid, fmode_t fmode)
{
write_seqlock(&state->seqlock);
- nfs_clear_open_stateid_locked(state, arg_stateid, stateid, fmode);
+ /* Ignore, if the CLOSE argment doesn't match the current stateid */
+ if (nfs4_state_match_open_stateid_other(state, arg_stateid))
+ nfs_clear_open_stateid_locked(state, stateid, fmode);
write_sequnlock(&state->seqlock);
if (test_bit(NFS_STATE_RECLAIM_NOGRACE, &state->flags))
nfs4_schedule_state_manager(state->owner->so_server->nfs_client);
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] Fix CLOSE races
From: Trond Myklebust @ 2016-11-14 16:19 UTC (permalink / raw)
To: linux-nfs; +Cc: Benjamin Coddington, Jeff Layton
Trond Myklebust (2):
NFSv4: Fix CLOSE races with OPEN
NFSv4: Don't call close if the open stateid has already been cleared
fs/nfs/nfs4_fs.h | 7 +++++++
fs/nfs/nfs4proc.c | 15 ++++++++-------
2 files changed, 15 insertions(+), 7 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH 4/4] sanity.bbclass: fix check_connectivity() for BB_NO_NETWORK = "0"
From: Robert Yang @ 2016-11-14 16:19 UTC (permalink / raw)
To: Christopher Larson; +Cc: Patches and discussions about the oe-core layer
In-Reply-To: <CABcZAN=v680FxZ5GHRN53hj3Rd5TnNhQxmVRMds0qZMW7Lc=bg@mail.gmail.com>
On 11/15/2016 12:08 AM, Christopher Larson wrote:
>
> On Mon, Nov 14, 2016 at 9:05 AM, Robert Yang <liezhi.yang@windriver.com
> <mailto:liezhi.yang@windriver.com>> wrote:
>
> On 11/14/2016 11:38 PM, Christopher Larson wrote:
>
>
> On Mon, Nov 14, 2016 at 8:37 AM, Robert Yang <liezhi.yang@windriver.com
> <mailto:liezhi.yang@windriver.com>
> <mailto:liezhi.yang@windriver.com <mailto:liezhi.yang@windriver.com>>>
> wrote:
>
> On 11/14/2016 11:03 PM, Christopher Larson wrote:
>
>
> On Mon, Nov 14, 2016 at 7:34 AM, Robert Yang
> <liezhi.yang@windriver.com <mailto:liezhi.yang@windriver.com>
> <mailto:liezhi.yang@windriver.com
> <mailto:liezhi.yang@windriver.com>>
> <mailto:liezhi.yang@windriver.com
> <mailto:liezhi.yang@windriver.com> <mailto:liezhi.yang@windriver.com
> <mailto:liezhi.yang@windriver.com>>>>
> wrote:
>
> The old code:
> network_enabled = not d.getVar('BB_NO_NETWORK', True)
>
> It is True only when BB_NO_NETWORK is not set (None),
> but BB_NO_NETWORK = "0" should also be True while "1" means
> no network,
> "0" means need network in a normal case.
>
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com
> <mailto:liezhi.yang@windriver.com>
> <mailto:liezhi.yang@windriver.com
> <mailto:liezhi.yang@windriver.com>>
> <mailto:liezhi.yang@windriver.com
> <mailto:liezhi.yang@windriver.com> <mailto:liezhi.yang@windriver.com
> <mailto:liezhi.yang@windriver.com>>>>
>
> ---
> meta/classes/sanity.bbclass | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/meta/classes/sanity.bbclass
> b/meta/classes/sanity.bbclass
> index 7e383f9..c5e3809 100644
> --- a/meta/classes/sanity.bbclass
> +++ b/meta/classes/sanity.bbclass
> @@ -363,15 +363,19 @@ def check_connectivity(d):
> test_uris = (d.getVar('CONNECTIVITY_CHECK_URIS', True) or
> "").split()
> retval = ""
>
> + bbn = d.getVar('BB_NO_NETWORK', True)
> + if bbn not in (None, '0', '1'):
> + return 'BB_NO_NETWORK should be "0" or "1", but it
> is "%s"'
> % bbn
>
>
> Does this mirror the same logic used in bitbake? What’s the
> behavior if it’s
> set, but to the empty string?
>
>
> bitbake only checks whether it equals "1" or not. Without this
> patch, an empty
> string is the same as not set since it doesn't equal to "1". But if
> it is
> set to "0", bitbake uses it as enable network, sanity.bbclass uses it
> as disable netowrk, which are conflicted. We can add checking for
> empty string,
> but do we have to ? Limit it to "0" or "1" makes things clear.
>
>
> IMO if we’re going to change the semantics, we should do it in bitbake
> and then
> mirror that in the metadata. Sanity checking should mirror the actual
> variable
> behavior where it’s used.
>
>
> Sounds reasonable, but I'm not sure how to do it, ways I can think out:
> 1) Handle "0" as enable network as bitbake does in sanity.bbclass
> 2) If we want to limit its values, maybe we need check it in bitbake rather
> than in sanity.bbclass, there are also other values have the similar
> problems, I did a rough grep, such as BB_FETCH_PREMIRRORONLY:
>
> fetch2/__init__.py: premirroronly =
> (self.d.getVar("BB_FETCH_PREMIRRORONLY", True) == "1")
> fetch2/git.py: if d.getVar("BB_FETCH_PREMIRRORONLY", True) is not None:
>
> The __init__.py only checks whether it is "1" or not, but git.py checks if it
> is None, there would be confusions when it is "" or "0".
>
>
> Sounds like bb.utils.to_boolean() may be our friend for a number of these.
Thanks, sounds good to me, let's see others comments, and I will work on that
later if no objections (maybe 1 month later). Currently I will simply make
sanity.bbclass handle "0" as bitbake does.
Have good day. I have to go to sleep now.
// Robert
> --
> Christopher Larson
> clarson at kergoth dot com
> Founder - BitBake, OpenEmbedded, OpenZaurus
> Maintainer - Tslib
> Senior Software Engineer, Mentor Graphics
^ permalink raw reply
* Re: [PATCH v3] ip6_output: ensure flow saddr actually belongs to device
From: David Ahern @ 2016-11-14 16:19 UTC (permalink / raw)
To: Jason A. Donenfeld, Netdev, WireGuard mailing list, LKML,
YOSHIFUJI Hideaki, Hannes Frederic Sowa
In-Reply-To: <20161113232813.28926-1-Jason@zx2c4.com>
On 11/13/16 4:28 PM, Jason A. Donenfeld wrote:
> This puts the IPv6 routing functions in parity with the IPv4 routing
> functions. Namely, we now check in v6 that if a flowi6 requests an
> saddr, the returned dst actually corresponds to a net device that has
> that saddr. This mirrors the v4 logic with __ip_dev_find in
> __ip_route_output_key_hash. In the event that the returned dst is not
> for a dst with a dev that has the saddr, we return -EINVAL, just like
> v4; this makes it easy to use the same error handlers for both cases.
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Cc: David Ahern <dsa@cumulusnetworks.com>
> ---
> Changes from v2:
> It turns out ipv6_chk_addr already has the device enumeration
> logic that we need by simply passing NULL.
>
> net/ipv6/ip6_output.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 6001e78..b3b5cb6 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -926,6 +926,10 @@ static int ip6_dst_lookup_tail(struct net *net, const struct sock *sk,
> int err;
> int flags = 0;
>
> + if (!ipv6_addr_any(&fl6->saddr) &&
> + !ipv6_chk_addr(net, &fl6->saddr, NULL, 1))
> + return -EINVAL;
> +
> /* The correct way to handle this would be to do
> * ip6_route_get_saddr, and then ip6_route_output; however,
> * the route-specific preferred source forces the
>
LGTM
Acked-by: David Ahern <dsa@cumulusnetworks.com>
^ permalink raw reply
* Re: [PATCH 1/3] idle: add support for tasks that inject idle
From: Jacob Pan @ 2016-11-14 16:20 UTC (permalink / raw)
To: Peter Zijlstra
Cc: LKML, Linux PM, Thomas Gleixner, Ingo Molnar, Zhang Rui,
Rafael Wysocki, Chen, Yu C, Sebastian Andrzej Siewior,
Petr Mladek, Srinivas Pandruvada, Arjan van de Ven, jacob.jun.pan
In-Reply-To: <20161114150119.GG3142@twins.programming.kicks-ass.net>
On Mon, 14 Nov 2016 16:01:19 +0100
Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Nov 09, 2016 at 11:05:10AM -0800, Jacob Pan wrote:
> > +void play_idle()
> > +{
> > + /*
> > + * Only FIFO tasks can disable the tick since they don't
> > need the forced
> > + * preemption.
> > + */
> > + WARN_ON_ONCE(current->policy != SCHED_FIFO);
> > + WARN_ON_ONCE(current->nr_cpus_allowed != 1);
> > + WARN_ON_ONCE(!(current->flags & PF_KTHREAD));
> > + WARN_ON_ONCE(!(current->flags & PF_NO_SETAFFINITY));
> > + rcu_sleep_check();
> > +
> > + preempt_disable();
> > + current->flags |= PF_IDLE;
> > + do_idle();
> > + current->flags &= ~PF_IDLE;
> > +
> > + preempt_fold_need_resched();
> > + preempt_enable();
> > +}
> > +EXPORT_SYMBOL_GPL(play_idle);
>
> Hurm.. didn't this initially also include the setup of the timer and
> take an timeout argument?
right, the initial version has a timeout and
init_timer_on_stack(timer);
I thought since this play_idle timer is likely to expire instead of
being canceled, so I switched to hrtimer by caller. We don't have an on
stack hrtimer, right? or Should we consider adding one?
Thanks,
Jacob
^ permalink raw reply
* Re: Understanding encrypted OSD's and cephx
From: Sage Weil @ 2016-11-14 16:17 UTC (permalink / raw)
To: Owen Synge; +Cc: Ceph Development
In-Reply-To: <7a406860-1bc6-7534-9e2c-341bb6ac09eb@suse.com>
On Mon, 14 Nov 2016, Owen Synge wrote:
> Dear all,
>
> I have been trying to get to grips with understanding cephx and
> encrypted OSD's. WRT this bug:
>
> http://tracker.ceph.com/issues/17833
>
> Please correct me here if I am wrong:
>
> Installing a encrypted OSD with ceph-deploy as:
>
> ceph-deploy osd create --dmcrypt ceph-node3:vdc
>
> Will fail unless you first run:
>
> ceph-deploy osd admin ceph-node3
>
> I have been digging further:
>
> (1) Encrypted OSD's are mounted using a key.
> (2) This key is retrieved from the mon, using a cephx key.
> (3) This cephx key is generated with the capability to retrieve the key
> from the mon.
>
> The reason being the admin key is needed is to make keys with correct
> capabilities.
>
> To set this up I have played with the command.
>
> With the admin key you can:
>
> /usr/bin/ceph --connect-timeout 20 config-key list
> /usr/bin/ceph --connect-timeout 20 config-key put key value
> /usr/bin/ceph --connect-timeout 20 config-key get key value
>
> So we dont want to put the admin keyring on the OSD's with encrypted
> OSD's as all encrypted OSD keys can be retrived using the admin keyring.
>
> So I then looked at locking down the capabilities:
>
> I can generate a key with only the ability to get a single key:
>
> /usr/bin/ceph auth get-or-create \
> client.osd-lockbox.d967ec85-4bd5-44c5-b20c-fc6864f6c7c0 \
> mon 'allow command "config-key get" with key="Key_name"' \
> > /tmp/foo
>
> I can then use this key to get the key's value:
>
> /usr/bin/ceph --keyring /tmp/foo --name \
> client.osd-lockbox.d967ec85-4bd5-44c5-b20c-fc6864f6c7c0 \
> config-key get Key_name
>
> I can also create a key that can only place values for a key:
>
> /usr/bin/ceph auth get-or-create client.bar mon \
> 'allow command "config-key put"' > /tmp/bar
>
> And this can upload to that value:
>
> /usr/bin/ceph auth get-or-create client.nting mon \
> 'allow command "config-key put" with \
> key="Key_name"'
>
> And it can only set this value:
>
> /usr/bin/ceph --connect-timeout 20 --keyring /tmp/bar --name \
> client.bar config-key put Key_name Key_value
>
> I can also use the mon keyrign ratehr than the admin key to create these
> keys:
>
> /usr/bin/ceph --connect-timeout 20 --keyring \
> /var/lib/ceph/mon/ceph-ceph-node1/keyring \
> --name mon. auth get-or-create client.jam mon \
> 'allow command "config-key put" with \
> key="Key_name"'
>
> So it seems to me, that we can potentially resolve bug:
>
> http://tracker.ceph.com/issues/17833
>
> Without ever putting the admin key or similar high privileged key on an
> encrypted OSD node.
>
> Now where does this leave us:
>
> (1) We either continue to expect the admin keyring to exist.
> (2) We use locked down keys on the OSD nodes.
>
> Following option (2) We can take 1 of 2 approaches:
>
> (A) ceph-deploy shoudl set up the encryption key and value on the mon
> node, generate a value readonly key to the OSD node then deploy the
> encrypted OSD.
> (B) ceph-deploy should set up a read and write keys for the OSD
> encryption value, then ship these to OSD node to deploy the OSD, then
> remove the write key.
>
> In both these cases ceph-deploy will need to contact the osd node, get
> the partion UUID, process this on the mon node, then finish the process
> of deploying an encrypted OSD on the OSD node.
>
> Option (A) seems simpler here.
>
> Does anyone see a better way assuming (2)?
I think ideally we want this to be permitted as part of the osd-bootstrap
key. Then we can hopefully improve things to make provisioning tools
temporarily issue a bootstrap key, create the osds, then remove the
bootstrep key.
However, it isn't really possible to sufficiently restrict the commands to
do the bootstrap with a generic key. You need to know things like the
uuid to restrict the key properly. And it's tedious. And even the
current bootstrap process isn't fully secure, see MonCap.cc:
if (profile == "bootstrap-mds") {
...
profile_grants.push_back(MonCapGrant("auth get-or-create")); // FIXME: this can expose other mds keys
I think the fix for this is to give up on trying to write a capability
that lets you do the necessary steps to set up an OSD or MDS or whatever,
and instead define a new monitor command to do all of this in one go.
For example, 'osd bootstrap' could take the necessary arguments (e.g., the
uuid and local key) and in one step set up the various keys for you. I
think that's going to be the most maintainable and sane solution going
forward.
In the meantime, though... let's just change the ceph deploy test to
install the admin key so that we can make the tests pass? And set teh bug
severity on this so that we make sure it's addressed in a complete way
soon.
sage
^ permalink raw reply
* Re: [PATCH] KVM: x86: do not go through vcpu in __get_kvmclock_ns
From: Paolo Bonzini @ 2016-11-14 16:17 UTC (permalink / raw)
To: Radim Krčmář; +Cc: linux-kernel, kvm, mtosatti
In-Reply-To: <20161114145239.GA2185@potion>
On 14/11/2016 15:52, Radim Krčmář wrote:
> The hunk below should return the same value in pvclock_ns and kernel_ns
> if they can be used interchangeably. boot_ns is expected to be a bit
> delayed, because it is read late. boot_ns shows a bounded offset from
> kernel_ns, unlike the drifting pvclock_ns.
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 83990ad3710e..30d4d3d02ac7 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6653,6 +6653,17 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> goto cancel_injection;
> }
>
> + if (vcpu->kvm->arch.use_master_clock) {
> + s64 kernel_ns;
> + cycle_t tsc_now, pvclock_ns, boot_ns;
> +
> + kvm_get_time_and_clockread(&kernel_ns, &tsc_now);
> + pvclock_ns = __pvclock_read_cycles(&vcpu->arch.hv_clock, kvm_read_l1_tsc(vcpu, tsc_now)) - vcpu->kvm->arch.kvmclock_offset;
> + boot_ns = ktime_get_boot_ns();
> +
> + printk("ns diff: %lld %lld\n", pvclock_ns - kernel_ns, boot_ns - kernel_ns);
> + }
> +
> preempt_disable();
>
> kvm_x86_ops->prepare_guest_switch(vcpu);
Ok, I'll post a v2 of this patch.
Paolo
^ permalink raw reply
* Re: [WireGuard] [PATCH v3] ip6_output: ensure flow saddr actually belongs to device
From: David Ahern @ 2016-11-14 16:19 UTC (permalink / raw)
To: Jason A. Donenfeld, Netdev, WireGuard mailing list, LKML,
YOSHIFUJI Hideaki, Hannes Frederic Sowa
In-Reply-To: <20161113232813.28926-1-Jason@zx2c4.com>
On 11/13/16 4:28 PM, Jason A. Donenfeld wrote:
> This puts the IPv6 routing functions in parity with the IPv4 routing
> functions. Namely, we now check in v6 that if a flowi6 requests an
> saddr, the returned dst actually corresponds to a net device that has
> that saddr. This mirrors the v4 logic with __ip_dev_find in
> __ip_route_output_key_hash. In the event that the returned dst is not
> for a dst with a dev that has the saddr, we return -EINVAL, just like
> v4; this makes it easy to use the same error handlers for both cases.
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Cc: David Ahern <dsa@cumulusnetworks.com>
> ---
> Changes from v2:
> It turns out ipv6_chk_addr already has the device enumeration
> logic that we need by simply passing NULL.
>
> net/ipv6/ip6_output.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 6001e78..b3b5cb6 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -926,6 +926,10 @@ static int ip6_dst_lookup_tail(struct net *net, const struct sock *sk,
> int err;
> int flags = 0;
>
> + if (!ipv6_addr_any(&fl6->saddr) &&
> + !ipv6_chk_addr(net, &fl6->saddr, NULL, 1))
> + return -EINVAL;
> +
> /* The correct way to handle this would be to do
> * ip6_route_get_saddr, and then ip6_route_output; however,
> * the route-specific preferred source forces the
>
LGTM
Acked-by: David Ahern <dsa@cumulusnetworks.com>
^ permalink raw reply
* Re: [PATCH 2/2] pci: Don't set RCB bit in LNKCTL if the upstream bridge hasn't
From: Don Dutile @ 2016-11-14 16:16 UTC (permalink / raw)
To: Johannes Thumshirn, Bjorn Helgaas
Cc: Bjorn Helgaas, linux-pci, linux-kernel, Alexander Graf,
Hannes Reinecke
In-Reply-To: <20161114115604.gzxjstjj7vb4ytno@linux-x5ow.site>
On 11/14/2016 06:56 AM, Johannes Thumshirn wrote:
> On Wed, Nov 09, 2016 at 11:11:40AM -0600, Bjorn Helgaas wrote:
>> Hi Johannes,
>>
>> On Wed, Nov 02, 2016 at 04:35:52PM -0600, Johannes Thumshirn wrote:
>>> The Read Completion Boundary (RCB) bit must only be set on a device or
>>> endpoint if it is set on the root complex.
>>>
>>> Certain BIOSes erroneously set the RCB Bit in their ACPI _HPX Tables
>>> even if it is not set on the root port. This is a violation to the PCIe
>>> Specification and is known to bring some Mellanox Connect-X 3 HCAs into
>>> a state where they can't map their firmware and go into error recovery.
>>>
>>> BIOS Information
>>> Vendor: IBM
>>> Version: -[A8E120CUS-1.30]-
>>> Release Date: 08/22/2016
>>
>> This seems like a pretty serious problem (sounds like maybe the HCA is
>> completely useless?)
>
> Correct.
>
>>
>> Can you point us at a bugzilla or other problem report? It's nice to
>> have details of what this looks like to a user, so people who trip
>> over this problem have a little more chance of finding the solution.
>
> As we already said, our bugzilla entry for this is not accessible from the
> outside, but I know Red Hat does have a bugzilla entry for the same issue as
> well. Maybe this is reachable from the outside (adding Don for this, as I know
> he has worked on this problem as well).
>
RHEL bz's are not accessible from the outside.
I suggest capturing the content of the RH bz issue and creating a k.o. bz
with the information.
>>
>> 7a1562d4f2d0 ("PCI: Apply _HPX Link Control settings to all devices
>> with a link") appeared in v3.18, so it's probably not a *new* problem,
>> so my guess is that this is v4.10 material.
>
> Yes 4.10 sounds good to me. I personally think, this problem hasn't
> materialized yet, as this is the kind of hardware you run on a rather /stable/
> kernel either you built on your own or get from an enterprise distribution and
> until recently these kernels haven't been updated to something newer than
> 3.18.
>
> Thanks,
> Johannes
>
^ permalink raw reply
* ✓ Fi.CI.BAT: success for Geminilake enabling (rev5)
From: Patchwork @ 2016-11-14 16:16 UTC (permalink / raw)
To: Ander Conselvan de Oliveira; +Cc: intel-gfx
In-Reply-To: <1478791400-21756-1-git-send-email-ander.conselvan.de.oliveira@intel.com>
== Series Details ==
Series: Geminilake enabling (rev5)
URL : https://patchwork.freedesktop.org/series/15118/
State : success
== Summary ==
Series 15118v5 Geminilake enabling
https://patchwork.freedesktop.org/api/1.0/series/15118/revisions/5/mbox/
fi-bdw-5557u total:244 pass:229 dwarn:0 dfail:0 fail:0 skip:15
fi-bsw-n3050 total:244 pass:204 dwarn:0 dfail:0 fail:0 skip:40
fi-bxt-t5700 total:244 pass:216 dwarn:0 dfail:0 fail:0 skip:28
fi-byt-j1900 total:244 pass:216 dwarn:0 dfail:0 fail:0 skip:28
fi-byt-n2820 total:244 pass:212 dwarn:0 dfail:0 fail:0 skip:32
fi-hsw-4770 total:244 pass:224 dwarn:0 dfail:0 fail:0 skip:20
fi-hsw-4770r total:244 pass:224 dwarn:0 dfail:0 fail:0 skip:20
fi-ilk-650 total:244 pass:191 dwarn:0 dfail:0 fail:0 skip:53
fi-ivb-3520m total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22
fi-ivb-3770 total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22
fi-kbl-7200u total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22
fi-skl-6260u total:244 pass:230 dwarn:0 dfail:0 fail:0 skip:14
fi-skl-6700hq total:244 pass:223 dwarn:0 dfail:0 fail:0 skip:21
fi-skl-6700k total:244 pass:222 dwarn:1 dfail:0 fail:0 skip:21
fi-skl-6770hq total:244 pass:230 dwarn:0 dfail:0 fail:0 skip:14
fi-snb-2520m total:244 pass:212 dwarn:0 dfail:0 fail:0 skip:32
fi-snb-2600 total:244 pass:211 dwarn:0 dfail:0 fail:0 skip:33
522cb36969544bb72e3215644c21fac2a60af418 drm-intel-nightly: 2016y-11m-14d-15h-21m-36s UTC integration manifest
5baa173 drm/i915/glk: Add Geminilake PCI IDs
bdf9462 drm/i915/glk: Introduce Geminilake platform definition
4749323 drm/i915: Create a common GEN9_LP_FEATURE.
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2984/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply
* Re: [PATCH v7 3/3] clk: qcom: Add A53 clock driver
From: Georgi Djakov @ 2016-11-14 16:15 UTC (permalink / raw)
To: Rob Herring
Cc: sboyd, mturquette, linux-clk, devicetree, mark.rutland,
linux-kernel, linux-arm-msm
In-Reply-To: <20161109182440.hjiv725vpsh5dctd@rob-hp-laptop>
On 11/09/2016 08:24 PM, Rob Herring wrote:
> On Mon, Oct 31, 2016 at 04:55:26PM +0200, Georgi Djakov wrote:
>> Add a driver for the A53 Clock Controller. It is a hardware block that
>> implements a combined mux and half integer divider functionality. It can
>> choose between a fixed-rate clock or the dedicated A53 PLL. The source
>> and the divider can be set both at the same time.
>>
>> This is required for enabling CPU frequency scaling on platforms like
>> MSM8916.
>>
>> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
>> ---
>> .../devicetree/bindings/clock/qcom,a53cc.txt | 23 ++++
>> drivers/clk/qcom/Kconfig | 8 ++
>> drivers/clk/qcom/Makefile | 1 +
>> drivers/clk/qcom/a53cc.c | 152 +++++++++++++++++++++
>> 4 files changed, 184 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53cc.txt
>> create mode 100644 drivers/clk/qcom/a53cc.c
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,a53cc.txt b/Documentation/devicetree/bindings/clock/qcom,a53cc.txt
>> new file mode 100644
>> index 000000000000..82d1634a2713
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,a53cc.txt
>> @@ -0,0 +1,23 @@
>> +Qualcomm A53 CPU Clock Controller Binding
>> +------------------------------------------------
>> +The A53 CPU Clock Controller is hardware, which provides a combined
>> +mux and divider functionality for the CPU clocks. It can choose between
>> +a fixed rate clock and the dedicated A53 PLL. This hardware block is used
>> +on platforms such as msm8916.
>> +
>> +Required properties :
>> +- compatible : shall contain:
>> +
>> + "qcom,a53cc-msm8916"
>
> Same comment on the ordering. With that, for the binding:
>
> Acked-by: Rob Herring <robh@kernel.org>
>
Thanks for reviewing, Rob. I am also going to make this a child node
as discussed here [1]. Please let me know if you have any comments.
[1]. https://lkml.org/lkml/2016/11/11/387
Thanks,
Georgi
^ permalink raw reply
* Re: [PATCH v3.1 15/15] xen/x86: setup PVHv2 Dom0 ACPI tables
From: Jan Beulich @ 2016-11-14 16:15 UTC (permalink / raw)
To: Roger Pau Monne; +Cc: Andrew Cooper, boris.ostrovsky, xen-devel
In-Reply-To: <1477731601-10926-16-git-send-email-roger.pau@citrix.com>
>>> On 29.10.16 at 11:00, <roger.pau@citrix.com> wrote:
> Also, regions marked as E820_ACPI or E820_NVS are identity mapped into Dom0
> p2m, plus any top-level ACPI tables that should be accessible to Dom0 and
> that don't reside in RAM regions. This is needed because some memory maps
> don't properly account for all the memory used by ACPI, so it's common to
> find ACPI tables in holes.
I question whether this behavior should be enabled by default. Not
having seen the code yet I also wonder whether these regions
shouldn't simply be added to the guest's E820 as E820_ACPI, which
should then result in them getting mapped without further special
casing.
> +static int __init hvm_add_mem_range(struct domain *d, uint64_t s, uint64_t e,
> + uint32_t type)
I see s and e being uint64_t, but I don't see why type can't be plain
unsigned int.
> +{
> + unsigned int i;
> +
> + for ( i = 0; i < d->arch.nr_e820; i++ )
> + {
> + uint64_t rs = d->arch.e820[i].addr;
> + uint64_t re = rs + d->arch.e820[i].size;
> +
> + if ( rs == e && d->arch.e820[i].type == type )
> + {
> + d->arch.e820[i].addr = s;
> + return 0;
> + }
> +
> + if ( re == s && d->arch.e820[i].type == type &&
> + (i + 1 == d->arch.nr_e820 || d->arch.e820[i + 1].addr >= e) )
I understand this to be overlap prevention, but there's no equivalent
in the earlier if(). Are you relying on the table being strictly sorted at
all times? If so, a comment should say so.
> + {
> + d->arch.e820[i].size += e - s;
> + return 0;
> + }
> +
> + if ( rs >= e )
> + break;
> +
> + if ( re > s )
> + return -ENOMEM;
I don't think ENOMEM is appropriate to signal an overlap. And don't
you need to reverse these last two if()s?
> @@ -2112,6 +2166,371 @@ static int __init hvm_setup_cpus(struct domain *d,
> paddr_t entry,
> return 0;
> }
>
> +static int __init acpi_count_intr_ov(struct acpi_subtable_header *header,
> + const unsigned long end)
> +{
> +
> + acpi_intr_overrrides++;
> + return 0;
> +}
> +
> +static int __init acpi_set_intr_ov(struct acpi_subtable_header *header,
> + const unsigned long end)
May I ask for "ov" to become at least "ovr" in all cases? Also stray
const.
> +{
> + struct acpi_madt_interrupt_override *intr =
> + container_of(header, struct acpi_madt_interrupt_override, header);
Yet missing const here.
> + ACPI_MEMCPY(intsrcovr, intr, sizeof(*intr));
Structure assignment (for type safety; also elsewhere)?
> +static int __init hvm_setup_acpi_madt(struct domain *d, paddr_t *addr)
> +{
> + struct acpi_table_madt *madt;
> + struct acpi_table_header *table;
> + struct acpi_madt_io_apic *io_apic;
> + struct acpi_madt_local_apic *local_apic;
> + struct vcpu *saved_current, *v = d->vcpu[0];
> + acpi_status status;
> + unsigned long size;
> + unsigned int i;
> + int rc;
> +
> + /* Count number of interrupt overrides in the MADT. */
> + acpi_table_parse_madt(ACPI_MADT_TYPE_INTERRUPT_OVERRIDE, acpi_count_intr_ov,
> + MAX_IRQ_SOURCES);
> +
> + /* Calculate the size of the crafted MADT. */
> + size = sizeof(struct acpi_table_madt);
> + size += sizeof(struct acpi_madt_interrupt_override) * acpi_intr_overrrides;
> + size += sizeof(struct acpi_madt_io_apic);
> + size += sizeof(struct acpi_madt_local_apic) * dom0_max_vcpus();
All the sizeof()s would better use the variables declared above.
> + madt = xzalloc_bytes(size);
> + if ( !madt )
> + {
> + printk("Unable to allocate memory for MADT table\n");
> + return -ENOMEM;
> + }
> +
> + /* Copy the native MADT table header. */
> + status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
> + if ( !ACPI_SUCCESS(status) )
> + {
> + printk("Failed to get MADT ACPI table, aborting.\n");
> + return -EINVAL;
> + }
> + ACPI_MEMCPY(madt, table, sizeof(*table));
> + madt->address = APIC_DEFAULT_PHYS_BASE;
You may also need to override table revision (at least it shouldn't end
up larger than what we know about).
> + /* Setup the IO APIC entry. */
> + if ( nr_ioapics > 1 )
> + printk("WARNING: found %d IO APICs, Dom0 will only have access to 1 emulated IO APIC\n",
> + nr_ioapics);
I've said elsewhere already that I think we should provide 1 vIO-APIC
per physical one.
> + io_apic = (struct acpi_madt_io_apic *)(madt + 1);
> + io_apic->header.type = ACPI_MADT_TYPE_IO_APIC;
> + io_apic->header.length = sizeof(*io_apic);
> + io_apic->id = 1;
> + io_apic->address = VIOAPIC_DEFAULT_BASE_ADDRESS;
> +
> + local_apic = (struct acpi_madt_local_apic *)(io_apic + 1);
> + for ( i = 0; i < dom0_max_vcpus(); i++ )
> + {
> + local_apic->header.type = ACPI_MADT_TYPE_LOCAL_APIC;
> + local_apic->header.length = sizeof(*local_apic);
> + local_apic->processor_id = i;
> + local_apic->id = i * 2;
> + local_apic->lapic_flags = ACPI_MADT_ENABLED;
> + local_apic++;
> + }
What about x2apic? And for lapic, do you limit vCPU count anywhere?
> + /* Setup interrupt overwrites. */
overrides
> +static bool __init hvm_acpi_table_allowed(const char *sig)
> +{
> + static const char __init banned_tables[][ACPI_NAME_SIZE] = {
> + ACPI_SIG_HPET, ACPI_SIG_SLIT, ACPI_SIG_SRAT, ACPI_SIG_MPST,
> + ACPI_SIG_PMTT, ACPI_SIG_MADT, ACPI_SIG_DMAR};
> + unsigned long pfn, nr_pages;
> + int i;
> +
> + for ( i = 0 ; i < ARRAY_SIZE(banned_tables); i++ )
> + if ( strncmp(sig, banned_tables[i], ACPI_NAME_SIZE) == 0 )
> + return false;
> +
> + /* Make sure table doesn't reside in a RAM region. */
> + pfn = PFN_DOWN(acpi_gbl_root_table_list.tables[i].address);
> + nr_pages = DIV_ROUND_UP(acpi_gbl_root_table_list.tables[i].length,
> + PAGE_SIZE);
You also need to add in the offset-into-page from the base address.
> + if ( range_is_ram(pfn, nr_pages) )
> + {
> + printk("Skipping table %.4s because resides in a RAM region\n",
> + sig);
> + return false;
I think this should be more strict, at least to start with: Require the
table to be in an E820_ACPI region (or maybe an E820_RESERVED
one), but nothing else.
> +static int __init hvm_setup_acpi_xsdt(struct domain *d, paddr_t madt_addr,
> + paddr_t *addr)
> +{
> + struct acpi_table_xsdt *xsdt;
> + struct acpi_table_header *table;
> + struct acpi_table_rsdp *rsdp;
> + struct vcpu *saved_current, *v = d->vcpu[0];
> + unsigned long size;
> + unsigned int i, num_tables;
> + int j, rc;
> +
> + /*
> + * Restore original DMAR table signature, we are going to filter it
> + * from the new XSDT that is presented to the guest, so it no longer
> + * makes sense to have it's signature zapped.
> + */
> + acpi_dmar_reinstate();
> +
> + /* Account for the space needed by the XSDT. */
> + size = sizeof(*xsdt);
> + num_tables = 0;
> +
> + /* Count the number of tables that will be added to the XSDT. */
> + for( i = 0; i < acpi_gbl_root_table_list.count; i++ )
> + {
> + const char *sig = acpi_gbl_root_table_list.tables[i].signature.ascii;
> +
> + if ( !hvm_acpi_table_allowed(sig) )
> + continue;
> +
> + num_tables++;
> + }
Unless you expect something to be added to this loop, please
simplify it by inverting the condition and dropping the continue.
> + /*
> + * No need to subtract one because we will be adding a custom MADT (and
> + * the native one is not accounted for).
> + */
> + size += num_tables * sizeof(u64);
sizeof(xsdt->table_offset_entry[0])
> + xsdt = xzalloc_bytes(size);
> + if ( !xsdt )
> + {
> + printk("Unable to allocate memory for XSDT table\n");
> + return -ENOMEM;
> + }
> +
> + /* Copy the native XSDT table header. */
> + rsdp = acpi_os_map_memory(acpi_os_get_root_pointer(), sizeof(*rsdp));
> + if ( !rsdp )
> + {
> + printk("Unable to map RSDP\n");
> + return -EINVAL;
> + }
> + table = acpi_os_map_memory(rsdp->xsdt_physical_address, sizeof(*table));
> + if ( !table )
> + {
> + printk("Unable to map XSDT\n");
> + return -EINVAL;
> + }
> + ACPI_MEMCPY(xsdt, table, sizeof(*table));
> + acpi_os_unmap_memory(table, sizeof(*table));
> + acpi_os_unmap_memory(rsdp, sizeof(*rsdp));
At this point we're not in SYS_STATE_active yet, and hence there
can only be one mapping at a time. The way it's written right now
does not represent an active problem, but to prevent someone
falling into this trap you should unmap the first mapping before
establishing the second one.
> + /* Add the custom MADT. */
> + j = 0;
> + xsdt->table_offset_entry[j++] = madt_addr;
> +
> + /* Copy the address of the rest of the allowed tables. */
addresses?
> + for( i = 0; i < acpi_gbl_root_table_list.count; i++ )
> + {
> + const char *sig = acpi_gbl_root_table_list.tables[i].signature.ascii;
> + unsigned long pfn, nr_pages;
> +
> + if ( !hvm_acpi_table_allowed(sig) )
> + continue;
> +
> + /* Make sure table doesn't reside in a RAM region. */
> + pfn = PFN_DOWN(acpi_gbl_root_table_list.tables[i].address);
> + nr_pages = DIV_ROUND_UP(acpi_gbl_root_table_list.tables[i].length,
> + PAGE_SIZE);
See above (and there appears to be at least one more further down).
> + /* Make sure table is mapped. */
> + rc = modify_identity_mmio(d, pfn, nr_pages, true);
> + if ( rc )
> + printk("Failed to map ACPI region [%#lx, %#lx) into Dom0 memory map\n",
> + pfn, pfn + nr_pages);
Isn't the comment for this code block meant to go ahead of the earlier
one, in place of the comment that's there (and looks wrong)?
> + xsdt->table_offset_entry[j++] =
> + acpi_gbl_root_table_list.tables[i].address;
> + }
> +
> + xsdt->header.length = size;
> + xsdt->header.checksum -= acpi_tb_checksum(ACPI_CAST_PTR(u8, xsdt), size);
> +
> + /* Place the new XSDT in guest memory space. */
> + if ( hvm_steal_ram(d, size, GB(4), addr) )
> + {
> + printk("Unable to find allocate guest RAM for XSDT\n");
"find" or "allocate"?
> + return -ENOMEM;
> + }
> +
> + /* Mark this region as E820_ACPI. */
> + if ( hvm_add_mem_range(d, *addr, *addr + size, E820_ACPI) )
> + printk("Unable to add XSDT region to memory map\n");
> +
> + saved_current = current;
> + set_current(v);
> + rc = hvm_copy_to_guest_phys(*addr, xsdt, size);
> + set_current(saved_current);
This pattern appears to be recurring - please make a helper function
(which then also eases possibly addressing my earlier remark
regarding that playing with current).
> + if ( rc != HVMCOPY_okay )
> + {
> + printk("Unable to copy XSDT into guest memory\n");
> + return -EFAULT;
> + }
> + xfree(xsdt);
> +
> + return 0;
> +}
> +
> +
> +static int __init hvm_setup_acpi(struct domain *d, paddr_t start_info)
Only one blank line between functions please.
> +{
> + struct vcpu *saved_current, *v = d->vcpu[0];
> + struct acpi_table_rsdp rsdp;
> + unsigned long pfn, nr_pages;
> + paddr_t madt_paddr, xsdt_paddr, rsdp_paddr;
> + unsigned int i;
> + int rc;
> +
> + /* Identity map ACPI e820 regions. */
> + for ( i = 0; i < d->arch.nr_e820; i++ )
> + {
> + if ( d->arch.e820[i].type != E820_ACPI &&
> + d->arch.e820[i].type != E820_NVS )
> + continue;
> +
> + pfn = PFN_DOWN(d->arch.e820[i].addr);
> + nr_pages = DIV_ROUND_UP(d->arch.e820[i].size, PAGE_SIZE);
> +
> + rc = modify_identity_mmio(d, pfn, nr_pages, true);
> + if ( rc )
> + {
> + printk(
> + "Failed to map ACPI region [%#lx, %#lx) into Dom0 memory map\n",
> + pfn, pfn + nr_pages);
> + return rc;
> + }
> + }
> +
> + rc = hvm_setup_acpi_madt(d, &madt_paddr);
> + if ( rc )
> + return rc;
> +
> + rc = hvm_setup_acpi_xsdt(d, madt_paddr, &xsdt_paddr);
> + if ( rc )
> + return rc;
Coming back to the initial comment: If you did the 1:1 mapping last
and if you added problematic ranges to the E820 map, you wouldn't
need to call modify_identity_mmio() in two places.
> + /* Craft a custom RSDP. */
> + memset(&rsdp, 0, sizeof(rsdp));
> + memcpy(&rsdp.signature, ACPI_SIG_RSDP, sizeof(rsdp.signature));
> + memcpy(&rsdp.oem_id, "XenVMM", sizeof(rsdp.oem_id));
Is that a good idea? I think Dom0 should get to see the real OEM.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
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.