* Re: [PATCH 1/2] tls: Fix CA certificate presence check
From: Denis Kenzior @ 2016-11-14 20:47 UTC (permalink / raw)
To: ell
In-Reply-To: <1479154592-8116-1-git-send-email-andrew.zaborowski@intel.com>
[-- Attachment #1: Type: text/plain, Size: 189 bytes --]
Hi Andrew,
On 11/14/2016 02:16 PM, Andrew Zaborowski wrote:
> ---
> ell/tls.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Applied, thanks.
Regards,
-Denis
^ permalink raw reply
* Re: [PATHCv10 1/2] usb: USB Type-C connector class
From: Guenter Roeck @ 2016-11-14 20:46 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Greg KH, Oliver Neukum, Felipe Balbi, Bin Gao, linux-kernel,
linux-usb
In-Reply-To: <20161114123235.GD22706@kuha.fi.intel.com>
On Mon, Nov 14, 2016 at 02:32:35PM +0200, Heikki Krogerus wrote:
> Hi Greg,
>
> On Mon, Nov 14, 2016 at 10:51:48AM +0100, Greg KH wrote:
> > On Mon, Sep 19, 2016 at 02:16:56PM +0300, Heikki Krogerus wrote:
> > > The purpose of USB Type-C connector class is to provide
> > > unified interface for the user space to get the status and
> > > basic information about USB Type-C connectors on a system,
> > > control over data role swapping, and when the port supports
> > > USB Power Delivery, also control over power role swapping
> > > and Alternate Modes.
> > >
> > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > > Tested-by: Guenter Roeck <linux@roeck-us.net>
> > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > ---
> > > Documentation/ABI/testing/sysfs-class-typec | 218 ++++++
> > > Documentation/usb/typec.txt | 103 +++
> > > MAINTAINERS | 9 +
> > > drivers/usb/Kconfig | 2 +
> > > drivers/usb/Makefile | 2 +
> > > drivers/usb/typec/Kconfig | 7 +
> > > drivers/usb/typec/Makefile | 1 +
> > > drivers/usb/typec/typec.c | 1075 +++++++++++++++++++++++++++
> > > include/linux/usb/typec.h | 252 +++++++
> > > 9 files changed, 1669 insertions(+)
> > > create mode 100644 Documentation/ABI/testing/sysfs-class-typec
> > > create mode 100644 Documentation/usb/typec.txt
> > > create mode 100644 drivers/usb/typec/Kconfig
> > > create mode 100644 drivers/usb/typec/Makefile
> > > create mode 100644 drivers/usb/typec/typec.c
> > > create mode 100644 include/linux/usb/typec.h
> >
[ ... ]
> > > +
> > > +int typec_connect(struct typec_port *port, struct typec_connection *con)
> > > +{
> > > + int ret;
> > > +
> > > + if (!con->partner && !con->cable)
> > > + return -EINVAL;
> > > +
> > > + port->connected = 1;
> > > + port->data_role = con->data_role;
> > > + port->pwr_role = con->pwr_role;
> > > + port->vconn_role = con->vconn_role;
> > > + port->pwr_opmode = con->pwr_opmode;
> > > +
> > > + kobject_uevent(&port->dev.kobj, KOBJ_CHANGE);
> >
> > This worries me. Who is listening for it? What will you do with it?
> > Shouldn't you just poll on an attribute file instead?
>
> Oliver! Did you need this or can we remove it?
>
> I remember I removed the "connected" attribute because you did not see
> any use for it at one point. I don't remember the reason exactly why?
>
The Android team tells me that they are currently using the udev events
to track port role changes, and to detect presence of port partner.
Also, there are plans to track changes on usbc*cable to differentiate
between cable attach vs. device being attached on the remote end.
What is the problem with using kobject_uevent() and thus presumably
udev events ?
Thanks,
Guenter
^ permalink raw reply
* [U-Boot] [PATCH v3 7/8] x86: efi: Add a hello world test program
From: Alexander Graf @ 2016-11-14 20:46 UTC (permalink / raw)
To: u-boot
In-Reply-To: <CAPnjgZ2FA82rrOZicCrdXfpSrRbL1K_kqpM336SqismwfSidrQ@mail.gmail.com>
On 14/11/2016 21:44, Simon Glass wrote:
> Hi Alex,
>
> On 11 November 2016 at 23:23, Alexander Graf <agraf@suse.de> wrote:
>>
>>
>>> Am 11.11.2016 um 17:17 schrieb Simon Glass <sjg@chromium.org>:
>>>
>>> Hi Alex,
>>>
>>>> On 7 November 2016 at 09:32, Alexander Graf <agraf@suse.de> wrote:
>>>>
>>>>
>>>>> On 07/11/2016 10:46, Simon Glass wrote:
>>>>>
>>>>> Hi Alex,
>>>>>
>>>>>> On 19 October 2016 at 01:09, Alexander Graf <agraf@suse.de> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On 18/10/2016 22:37, Simon Glass wrote:
>>>>>>>
>>>>>>> Hi Alex,
>>>>>>>
>>>>>>>> On 18 October 2016 at 01:14, Alexander Graf <agraf@suse.de> wrote:
>>>>>>>>
>>>>>>>>> On 10/18/2016 04:29 AM, Simon Glass wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> It is useful to have a basic sanity check for EFI loader support. Add
>>>>>>>>> a
>>>>>>>>> 'bootefi hello' command which loads HelloWord.efi and runs it under
>>>>>>>>> U-Boot.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> Changes in v3:
>>>>>>>>> - Include a link to the program instead of adding it to the tree
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> So, uh, where is the link?
>>>>>>>
>>>>>>>
>>>>>>> I put it in the README (see the arm patch).
>>>>>>>
>>>>>>>>
>>>>>>>> I'm really not convinced this command buys us anything yet. I do agree
>>>>>>>> that
>>>>>>>> we want automated testing - but can't we get that using QEMU and a
>>>>>>>> downloadable image file that we pass in as disk and have the distro
>>>>>>>> boot do
>>>>>>>> its magic?
>>>>>>>
>>>>>>>
>>>>>>> That seems very heavyweight as a sanity check, although I agree it is
>>>>>>> useful.
>>>>>>
>>>>>>
>>>>>> It's not really much more heavy weight. The "image file" could simply
>>>>>> contain your hello world binary. But with this we don't just verify
>>>>>> whether "bootefi" works, but also whether the default boot path works ok.
>>>>>
>>>>>
>>>>> I don't think I understand what you mean by 'image file'. Is it
>>>>> something other than the .efi file? Do you mean a disk image?
>>>>
>>>>
>>>> Yes. For reasonable test coverage, we should also verify that the distro
>>>> defaults wrote a sane boot script that automatically searches for a default
>>>> EFI binary in /efi/boot/bootx86.efi on the first partition of all devices
>>>> and runs it.
>>>>
>>>> So if we just provide an SD card image or hard disk image to QEMU which
>>>> contains a hello world .efi binary as that default boot file, we don't only
>>>> test whether the "bootefi" command works, but also whether the distro boot
>>>> script works.
>>>
>>> That's right.
>>>
>>>>
>>>>>
>>>>>>
>>>>>>> Here I am just making sure that EFI programs can start, print output
>>>>>>> and exit. It is a test that we can easily run without a lot of
>>>>>>> overhead, much less than a full distro boot.
>>>>>>
>>>>>>
>>>>>> Again, I don't think it's much more overhead and I do believe it gives
>>>>>> us much cleaner separation between responsibilities of code (tests go
>>>>>> where tests are).
>>>>>
>>>>>
>>>>> You are talking about a functional test, something that tests things
>>>>> end to end. I prefer to at least start with a smaller test. Granted it
>>>>> takes a little more work but it means there are fewer things to hunt
>>>>> through when something goes wrong.
>>>>
>>>>
>>>> Yes, I personally find unit tests terribly annoying and unproductive and
>>>> functional tests very helpful :). And in this case, the effort to write it
>>>> is about the same for both, just that the functional test actually tells you
>>>> that things work or don't work at the end of the day.
>>>>
>>>> With a code base like U-Boot, a simple functional test like the above plus
>>>> git bisect should get you to an offending patch very quickly.
>>>
>>> This is not a unit test - in fact the EFI stuff has no unit tests. I
>>> suppose if we are trying to find a name this is a small functional
>>> test since it exercises the general functionality.
>>>
>>> I am much keener on small tests than large ones for finding simple
>>> bugs. Of course you can generally bisect to find a bug, but the more
>>> layers of software you need to look for the harder this is.
>>>
>>> We could definitely use a pytest which checks an EFI boot into an
>>> image, but I don't think this obviates the need for a smaller targeted
>>> test like this one.
>>
>> I think arguing over this is moot :). More tests is usually a good thing, so whoever gets to write them gets to push them ;). As long as the licenses are sound at least.
>
> OK good, well please can you review this at some point?
Review what exactly?
> Also, are you
> planning to write the 'larger' test? How do you test this all in suse?
Planning yes, but I'm very good at not writing tests :).
Currently I'm testing this all in suse by running systems which rely on
the code to work.
Alex
^ permalink raw reply
* Re: [Qemu-devel] [PATCH for-2.8 0/2] pc: remove redundant fw_cfg file "etc/boot-cpus"
From: Michael S. Tsirkin @ 2016-11-14 20:46 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, Eduardo Habkost, Stefan Hajnoczi, Kevin O'Connor,
Gerd Hoffmann, Laszlo Ersek
In-Reply-To: <1478877672-73759-1-git-send-email-imammedo@redhat.com>
On Fri, Nov 11, 2016 at 04:21:10PM +0100, Igor Mammedov wrote:
>
> Commit 080ac219cc7d9c55adf925c3545b7450055ad625
> pc: Add 'etc/boot-cpus' fw_cfg file for machine with more than 255 CPUs
>
> added "etc/boot-cpus" fw_cfg file durung 2.8 merge window, however
> QEMU alredy had similar legacy FW_CFG_NB_CPUS fw_cfg entry that
> should do practically the same. Considering FW_CFG_NB_CPUS's been
> around for a long time and is used by external projects (firmwares)
> we can't replace it with 'etc/boot-cpus' fw_cfg file.
>
> Drop redundant 'etc/boot-cpus' fw_cfg file and reuse FW_CFG_NB_CPUS
> instead.
>
> So here goes QEMU part of fixup
I agree we shouldn't commit to a bad host/guest API
but I think we need to format it differently.
First revert the boot-cpus patch for 2.8.
On top of that, add a patch fixing FW_CFG_NB_CPUS.
> CC: Eduardo Habkost <ehabkost@redhat.com>
> CC: mst@redhat.com
> CC: Stefan Hajnoczi <stefanha@gmail.com>
> CC: "Kevin O'Connor" <kevin@koconnor.net>
> CC: Gerd Hoffmann <kraxel@redhat.com>
> CC: Laszlo Ersek <lersek@redhat.com>
>
> Igor Mammedov (2):
> fw_cfg: move FW_CFG_NB_CPUS out of fw_cfg_init1()
> pc: drop "etc/boot-cpus" fw_cfg file and use FW_CFG_NB_CPUS instead
>
> include/hw/i386/pc.h | 4 ++--
> hw/arm/virt.c | 4 +++-
> hw/i386/pc.c | 20 ++++++++------------
> hw/nvram/fw_cfg.c | 1 -
> hw/ppc/mac_newworld.c | 1 +
> hw/ppc/mac_oldworld.c | 1 +
> hw/sparc/sun4m.c | 1 +
> hw/sparc64/sun4u.c | 1 +
> 8 files changed, 17 insertions(+), 16 deletions(-)
>
> --
> 2.7.4
^ permalink raw reply
* [PATCH net][v2] bpf: fix range arithmetic for bpf map access
From: Josef Bacik @ 2016-11-14 20:45 UTC (permalink / raw)
To: jannh, ast, daniel, davem, netdev
I made some invalid assumptions with BPF_AND and BPF_MOD that could result in
invalid accesses to bpf map entries. Fix this up by doing a few things
1) Kill BPF_MOD support. This doesn't actually get used by the compiler in real
life and just adds extra complexity.
2) Fix the logic for BPF_AND, don't allow AND of negative numbers and set the
minimum value to 0 for positive AND's.
3) Don't do operations on the ranges if they are set to the limits, as they are
by definition undefined, and allowing arithmetic operations on those values
could make them appear valid when they really aren't.
This fixes the testcase provided by Jann as well as a few other theoretical
problems.
Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
V1->V2:
- set the MIN_RANGE to -1 to essentially disable all negative values for the min
value.
- rebased onto net instead of net-next.
include/linux/bpf_verifier.h | 5 ++--
kernel/bpf/verifier.c | 70 +++++++++++++++++++++++++++++---------------
2 files changed, 50 insertions(+), 25 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 7035b99..6aaf425 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -14,7 +14,7 @@
* are obviously wrong for any sort of memory access.
*/
#define BPF_REGISTER_MAX_RANGE (1024 * 1024 * 1024)
-#define BPF_REGISTER_MIN_RANGE -(1024 * 1024 * 1024)
+#define BPF_REGISTER_MIN_RANGE -1
struct bpf_reg_state {
enum bpf_reg_type type;
@@ -22,7 +22,8 @@ struct bpf_reg_state {
* Used to determine if any memory access using this register will
* result in a bad access.
*/
- u64 min_value, max_value;
+ s64 min_value;
+ u64 max_value;
union {
/* valid when type == CONST_IMM | PTR_TO_STACK | UNKNOWN_VALUE */
s64 imm;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 99a7e5b..6a93615 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -216,8 +216,8 @@ static void print_verifier_state(struct bpf_verifier_state *state)
reg->map_ptr->key_size,
reg->map_ptr->value_size);
if (reg->min_value != BPF_REGISTER_MIN_RANGE)
- verbose(",min_value=%llu",
- (unsigned long long)reg->min_value);
+ verbose(",min_value=%lld",
+ (long long)reg->min_value);
if (reg->max_value != BPF_REGISTER_MAX_RANGE)
verbose(",max_value=%llu",
(unsigned long long)reg->max_value);
@@ -758,7 +758,7 @@ static int check_mem_access(struct bpf_verifier_env *env, u32 regno, int off,
* index'es we need to make sure that whatever we use
* will have a set floor within our range.
*/
- if ((s64)reg->min_value < 0) {
+ if (reg->min_value < 0) {
verbose("R%d min value is negative, either use unsigned index or do a if (index >=0) check.\n",
regno);
return -EACCES;
@@ -1468,7 +1468,8 @@ static void check_reg_overflow(struct bpf_reg_state *reg)
{
if (reg->max_value > BPF_REGISTER_MAX_RANGE)
reg->max_value = BPF_REGISTER_MAX_RANGE;
- if ((s64)reg->min_value < BPF_REGISTER_MIN_RANGE)
+ if (reg->min_value < BPF_REGISTER_MIN_RANGE ||
+ reg->min_value > BPF_REGISTER_MAX_RANGE)
reg->min_value = BPF_REGISTER_MIN_RANGE;
}
@@ -1476,7 +1477,8 @@ static void adjust_reg_min_max_vals(struct bpf_verifier_env *env,
struct bpf_insn *insn)
{
struct bpf_reg_state *regs = env->cur_state.regs, *dst_reg;
- u64 min_val = BPF_REGISTER_MIN_RANGE, max_val = BPF_REGISTER_MAX_RANGE;
+ s64 min_val = BPF_REGISTER_MIN_RANGE;
+ u64 max_val = BPF_REGISTER_MAX_RANGE;
bool min_set = false, max_set = false;
u8 opcode = BPF_OP(insn->code);
@@ -1512,22 +1514,43 @@ static void adjust_reg_min_max_vals(struct bpf_verifier_env *env,
return;
}
+ /* If one of our values was at the end of our ranges then we can't just
+ * do our normal operations to the register, we need to set the values
+ * to the min/max since they are undefined.
+ */
+ if (min_val == BPF_REGISTER_MIN_RANGE)
+ dst_reg->min_value = BPF_REGISTER_MIN_RANGE;
+ if (max_val == BPF_REGISTER_MAX_RANGE)
+ dst_reg->max_value = BPF_REGISTER_MAX_RANGE;
+
switch (opcode) {
case BPF_ADD:
- dst_reg->min_value += min_val;
- dst_reg->max_value += max_val;
+ if (dst_reg->min_value != BPF_REGISTER_MIN_RANGE)
+ dst_reg->min_value += min_val;
+ if (dst_reg->max_value != BPF_REGISTER_MAX_RANGE)
+ dst_reg->max_value += max_val;
break;
case BPF_SUB:
- dst_reg->min_value -= min_val;
- dst_reg->max_value -= max_val;
+ if (dst_reg->min_value != BPF_REGISTER_MIN_RANGE)
+ dst_reg->min_value -= min_val;
+ if (dst_reg->max_value != BPF_REGISTER_MAX_RANGE)
+ dst_reg->max_value -= max_val;
break;
case BPF_MUL:
- dst_reg->min_value *= min_val;
- dst_reg->max_value *= max_val;
+ if (dst_reg->min_value != BPF_REGISTER_MIN_RANGE)
+ dst_reg->min_value *= min_val;
+ if (dst_reg->max_value != BPF_REGISTER_MAX_RANGE)
+ dst_reg->max_value *= max_val;
break;
case BPF_AND:
- /* & is special since it could end up with 0 bits set. */
- dst_reg->min_value &= min_val;
+ /* Disallow AND'ing of negative numbers, ain't nobody got time
+ * for that. Otherwise the minimum is 0 and the max is the max
+ * value we could AND against.
+ */
+ if (min_val < 0)
+ dst_reg->min_value = BPF_REGISTER_MIN_RANGE;
+ else
+ dst_reg->min_value = 0;
dst_reg->max_value = max_val;
break;
case BPF_LSH:
@@ -1537,24 +1560,25 @@ static void adjust_reg_min_max_vals(struct bpf_verifier_env *env,
*/
if (min_val > ilog2(BPF_REGISTER_MAX_RANGE))
dst_reg->min_value = BPF_REGISTER_MIN_RANGE;
- else
+ else if (dst_reg->min_value != BPF_REGISTER_MIN_RANGE)
dst_reg->min_value <<= min_val;
if (max_val > ilog2(BPF_REGISTER_MAX_RANGE))
dst_reg->max_value = BPF_REGISTER_MAX_RANGE;
- else
+ else if (dst_reg->max_value != BPF_REGISTER_MAX_RANGE)
dst_reg->max_value <<= max_val;
break;
case BPF_RSH:
- dst_reg->min_value >>= min_val;
- dst_reg->max_value >>= max_val;
- break;
- case BPF_MOD:
- /* % is special since it is an unsigned modulus, so the floor
- * will always be 0.
+ /* RSH by a negative number is undefined, and the BPF_RSH is an
+ * unsigned shift, so make the appropriate casts.
*/
- dst_reg->min_value = 0;
- dst_reg->max_value = max_val - 1;
+ if (min_val < 0 || dst_reg->min_value < 0)
+ dst_reg->min_value = BPF_REGISTER_MIN_RANGE;
+ else
+ dst_reg->min_value =
+ (u64)(dst_reg->min_value) >> min_val;
+ if (dst_reg->max_value != BPF_REGISTER_MAX_RANGE)
+ dst_reg->max_value >>= max_val;
break;
default:
reset_reg_range_values(regs, insn->dst_reg);
--
2.5.5
^ permalink raw reply related
* Re: [PATCH] f2fs: fix to account total free nid correctly
From: Jaegeuk Kim @ 2016-11-14 20:45 UTC (permalink / raw)
To: Chao Yu; +Cc: chao, linux-kernel, linux-f2fs-devel
In-Reply-To: <20161114112456.113074-1-yuchao0@huawei.com>
On Mon, Nov 14, 2016 at 07:24:56PM +0800, Chao Yu wrote:
> Thread A Thread B Thread C
> - f2fs_create
> - f2fs_new_inode
> - f2fs_lock_op
> - alloc_nid
> alloc last nid
> - f2fs_unlock_op
> - f2fs_create
> - f2fs_new_inode
> - f2fs_lock_op
> - alloc_nid
> as node count still not
> be increased, we will
> loop in alloc_nid
> - f2fs_write_node_pages
> - f2fs_balance_fs_bg
> - f2fs_sync_fs
> - write_checkpoint
> - block_operations
> - f2fs_lock_all
> - f2fs_lock_op
>
> While creating new inode, we do not allocate and account nid atomically,
> so that when there is almost no free nids left, we may encounter deadloop
> like above stack.
>
> In order to avoid that, add nm_i::free_nid_cnt for accounting free nids
> and do nid allocation atomically during node creation.
How about using nm_i::avaiable_nids for this?
It seems that we don't need both of variables at the same time.
Thanks,
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/f2fs.h | 1 +
> fs/f2fs/node.c | 19 +++++++++++++++----
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 6de1fbf..9de6f20 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -551,6 +551,7 @@ struct f2fs_nm_info {
> struct radix_tree_root free_nid_root;/* root of the free_nid cache */
> struct list_head nid_list[MAX_NID_LIST];/* lists for free nids */
> unsigned int nid_cnt[MAX_NID_LIST]; /* the number of free node id */
> + unsigned int free_nid_cnt; /* the number of total free nid */
> spinlock_t nid_list_lock; /* protect nid lists ops */
> struct mutex build_lock; /* lock for build free nids */
>
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index d58438f..e412d0e 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1885,11 +1885,13 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
> return false;
> }
> #endif
> - if (unlikely(sbi->total_valid_node_count + 1 > nm_i->available_nids))
> - return false;
> -
> spin_lock(&nm_i->nid_list_lock);
>
> + if (unlikely(nm_i->free_nid_cnt == 0)) {
> + spin_unlock(&nm_i->nid_list_lock);
> + return false;
> + }
> +
> /* We should not use stale free nids created by build_free_nids */
> if (nm_i->nid_cnt[FREE_NID_LIST] && !on_build_free_nids(nm_i)) {
> f2fs_bug_on(sbi, list_empty(&nm_i->nid_list[FREE_NID_LIST]));
> @@ -1900,6 +1902,7 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
> __remove_nid_from_list(sbi, i, FREE_NID_LIST, true);
> i->state = NID_ALLOC;
> __insert_nid_to_list(sbi, i, ALLOC_NID_LIST, false);
> + nm_i->free_nid_cnt--;
> spin_unlock(&nm_i->nid_list_lock);
> return true;
> }
> @@ -1951,6 +1954,9 @@ void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
> i->state = NID_NEW;
> __insert_nid_to_list(sbi, i, FREE_NID_LIST, false);
> }
> +
> + nm_i->free_nid_cnt++;
> +
> spin_unlock(&nm_i->nid_list_lock);
>
> if (need_free)
> @@ -2222,8 +2228,12 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
> raw_nat_from_node_info(raw_ne, &ne->ni);
> nat_reset_flag(ne);
> __clear_nat_cache_dirty(NM_I(sbi), ne);
> - if (nat_get_blkaddr(ne) == NULL_ADDR)
> + if (nat_get_blkaddr(ne) == NULL_ADDR) {
> add_free_nid(sbi, nid, false);
> + spin_lock(&NM_I(sbi)->nid_list_lock);
> + NM_I(sbi)->free_nid_cnt++;
> + spin_unlock(&NM_I(sbi)->nid_list_lock);
> + }
> }
>
> if (to_journal)
> @@ -2302,6 +2312,7 @@ static int init_node_manager(struct f2fs_sb_info *sbi)
> nm_i->nid_cnt[FREE_NID_LIST] = 0;
> nm_i->nid_cnt[ALLOC_NID_LIST] = 0;
> nm_i->nat_cnt = 0;
> + nm_i->free_nid_cnt = nm_i->available_nids - sbi->total_valid_node_count;
> nm_i->ram_thresh = DEF_RAM_THRESHOLD;
> nm_i->ra_nid_pages = DEF_RA_NID_PAGES;
> nm_i->dirty_nats_ratio = DEF_DIRTY_NAT_RATIO_THRESHOLD;
> --
> 2.8.2.311.gee88674
------------------------------------------------------------------------------
^ permalink raw reply
* Re: [PATCH] libselinux: fix subdir build and usage of cmdline CFLAGS
From: Jason Zaman @ 2016-11-14 20:46 UTC (permalink / raw)
To: Roberts, William C; +Cc: Stephen Smalley, selinux@tycho.nsa.gov
In-Reply-To: <476DC76E7D1DF2438D32BFADF679FC561CD24A31@ORSMSX103.amr.corp.intel.com>
On Mon, Nov 14, 2016 at 06:31:09PM +0000, Roberts, William C wrote:
>
>
> > -----Original Message-----
> > From: Stephen Smalley [mailto:sds@tycho.nsa.gov]
> > Sent: Monday, November 14, 2016 10:12 AM
> > To: selinux@tycho.nsa.gov
> > Cc: Roberts, William C <william.c.roberts@intel.com>; Stephen Smalley
> > <sds@tycho.nsa.gov>
> > Subject: [PATCH] libselinux: fix subdir build and usage of cmdline CFLAGS
> >
> > commit 16c123f4b1f3c8d20b3f597df161d7e635620923 ("libselinux:
> > support ANDROID_HOST=1 on Mac") broke the ability to run make in the src
> > subdirectory of libselinux (because OS and COMPILER were not defined) and also
> > caused some warning flags that could be overridden via command-line CFLAGS to
> > be mandatory. Fix it.
>
> Ack.
>
> It would be nice if we could avoid the duplication running repent throughout these make files,
> maybe include a file that has the definitions/callable functions for all of this stuff?
I just noticed some weirdness in a bunch of the makefiles too. Some of
the variables randomly default to different things depending on the
package. including a base makefile would be nice but the problem is when
packaged up they are separate tarballs.
We'd probably need to unify everything and duplicate it in each of the
subdirs and then the release script should diff them and make sure there
are no discrepancies before release.
>
> >
> > Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> > ---
> > libselinux/src/Makefile | 12 ++++++++++--
> > libselinux/utils/Makefile | 10 +++++++++-
> > 2 files changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile index
> > 24946ce..76efe49 100644
> > --- a/libselinux/src/Makefile
> > +++ b/libselinux/src/Makefile
> > @@ -23,6 +23,14 @@ LIBSEPOLA ?= $(LIBDIR)/libsepol.a VERSION = $(shell cat
> > ../VERSION) LIBVERSION = 1
> >
> > +OS ?= $(shell uname)
> > +
> > +ifeq ($(shell $(CC) -v 2>&1 | grep "clang"),) COMPILER ?= gcc else
> > +COMPILER ?= clang endif
> > +
> > LIBA=libselinux.a
> > TARGET=libselinux.so
> > LIBPC=libselinux.pc
> > @@ -67,12 +75,12 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-
> > security -Winit-self -Wmissi LD_SONAME_FLAGS=-soname,$(LIBSO),-z,defs,-
> > z,relro
> >
> > ifeq ($(COMPILER), gcc)
> > -override CFLAGS += -fipa-pure-const -Wlogical-op -Wpacked-bitfield-compat -
> > Wsync-nand \
> > +CFLAGS += -fipa-pure-const -Wlogical-op -Wpacked-bitfield-compat
> > +-Wsync-nand \
> > -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc -
> > Wsuggest-attribute=const \
> > -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wtrampolines
> > -Wjump-misses-init \
> > -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -Wp,-
> > D_FORTIFY_SOURCE=2 else -override CFLAGS += -Wunused-command-line-
> > argument
> > +CFLAGS += -Wunused-command-line-argument
> > endif
> >
> > ifeq ($(OS), Darwin)
> > diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile index
> > a4f9903..7744184 100644
> > --- a/libselinux/utils/Makefile
> > +++ b/libselinux/utils/Makefile
> > @@ -5,6 +5,14 @@ USRBINDIR ?= $(PREFIX)/sbin SBINDIR ?= $(DESTDIR)/sbin
> > INCLUDEDIR ?= $(PREFIX)/include
> >
> > +OS ?= $(shell uname)
> > +
> > +ifeq ($(shell $(CC) -v 2>&1 | grep "clang"),) COMPILER ?= gcc else
> > +COMPILER ?= clang endif
> > +
> > MAX_STACK_SIZE=8192
> > CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -
> > Wmissing-include-dirs \
> > -Wunused -Wunknown-pragmas -Wstrict-aliasing -Wshadow -Wpointer-
> > arith \ @@ -26,7 +34,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -
> > Wformat-security -Winit-self -Wmissi LD_SONAME_FLAGS=-soname,$(LIBSO),-
> > z,defs,-z,relro
> >
> > ifeq ($(COMPILER), gcc)
> > -override CFLAGS += -fipa-pure-const -Wpacked-bitfield-compat -Wsync-nand -
> > Wcoverage-mismatch \
> > +CFLAGS += -fipa-pure-const -Wpacked-bitfield-compat -Wsync-nand
> > +-Wcoverage-mismatch \
> > -Wcpp -Wformat-contains-nul -Wnormalized=nfc -Wsuggest-
> > attribute=const \
> > -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wtrampolines
> > -Wjump-misses-init \
> > -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> > --
> > 2.7.4
>
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
^ permalink raw reply
* Re: [PATCH] f2fs: fix to account total free nid correctly
From: Jaegeuk Kim @ 2016-11-14 20:45 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao
In-Reply-To: <20161114112456.113074-1-yuchao0@huawei.com>
On Mon, Nov 14, 2016 at 07:24:56PM +0800, Chao Yu wrote:
> Thread A Thread B Thread C
> - f2fs_create
> - f2fs_new_inode
> - f2fs_lock_op
> - alloc_nid
> alloc last nid
> - f2fs_unlock_op
> - f2fs_create
> - f2fs_new_inode
> - f2fs_lock_op
> - alloc_nid
> as node count still not
> be increased, we will
> loop in alloc_nid
> - f2fs_write_node_pages
> - f2fs_balance_fs_bg
> - f2fs_sync_fs
> - write_checkpoint
> - block_operations
> - f2fs_lock_all
> - f2fs_lock_op
>
> While creating new inode, we do not allocate and account nid atomically,
> so that when there is almost no free nids left, we may encounter deadloop
> like above stack.
>
> In order to avoid that, add nm_i::free_nid_cnt for accounting free nids
> and do nid allocation atomically during node creation.
How about using nm_i::avaiable_nids for this?
It seems that we don't need both of variables at the same time.
Thanks,
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/f2fs.h | 1 +
> fs/f2fs/node.c | 19 +++++++++++++++----
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 6de1fbf..9de6f20 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -551,6 +551,7 @@ struct f2fs_nm_info {
> struct radix_tree_root free_nid_root;/* root of the free_nid cache */
> struct list_head nid_list[MAX_NID_LIST];/* lists for free nids */
> unsigned int nid_cnt[MAX_NID_LIST]; /* the number of free node id */
> + unsigned int free_nid_cnt; /* the number of total free nid */
> spinlock_t nid_list_lock; /* protect nid lists ops */
> struct mutex build_lock; /* lock for build free nids */
>
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index d58438f..e412d0e 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1885,11 +1885,13 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
> return false;
> }
> #endif
> - if (unlikely(sbi->total_valid_node_count + 1 > nm_i->available_nids))
> - return false;
> -
> spin_lock(&nm_i->nid_list_lock);
>
> + if (unlikely(nm_i->free_nid_cnt == 0)) {
> + spin_unlock(&nm_i->nid_list_lock);
> + return false;
> + }
> +
> /* We should not use stale free nids created by build_free_nids */
> if (nm_i->nid_cnt[FREE_NID_LIST] && !on_build_free_nids(nm_i)) {
> f2fs_bug_on(sbi, list_empty(&nm_i->nid_list[FREE_NID_LIST]));
> @@ -1900,6 +1902,7 @@ bool alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid)
> __remove_nid_from_list(sbi, i, FREE_NID_LIST, true);
> i->state = NID_ALLOC;
> __insert_nid_to_list(sbi, i, ALLOC_NID_LIST, false);
> + nm_i->free_nid_cnt--;
> spin_unlock(&nm_i->nid_list_lock);
> return true;
> }
> @@ -1951,6 +1954,9 @@ void alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid)
> i->state = NID_NEW;
> __insert_nid_to_list(sbi, i, FREE_NID_LIST, false);
> }
> +
> + nm_i->free_nid_cnt++;
> +
> spin_unlock(&nm_i->nid_list_lock);
>
> if (need_free)
> @@ -2222,8 +2228,12 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
> raw_nat_from_node_info(raw_ne, &ne->ni);
> nat_reset_flag(ne);
> __clear_nat_cache_dirty(NM_I(sbi), ne);
> - if (nat_get_blkaddr(ne) == NULL_ADDR)
> + if (nat_get_blkaddr(ne) == NULL_ADDR) {
> add_free_nid(sbi, nid, false);
> + spin_lock(&NM_I(sbi)->nid_list_lock);
> + NM_I(sbi)->free_nid_cnt++;
> + spin_unlock(&NM_I(sbi)->nid_list_lock);
> + }
> }
>
> if (to_journal)
> @@ -2302,6 +2312,7 @@ static int init_node_manager(struct f2fs_sb_info *sbi)
> nm_i->nid_cnt[FREE_NID_LIST] = 0;
> nm_i->nid_cnt[ALLOC_NID_LIST] = 0;
> nm_i->nat_cnt = 0;
> + nm_i->free_nid_cnt = nm_i->available_nids - sbi->total_valid_node_count;
> nm_i->ram_thresh = DEF_RAM_THRESHOLD;
> nm_i->ra_nid_pages = DEF_RA_NID_PAGES;
> nm_i->dirty_nats_ratio = DEF_DIRTY_NAT_RATIO_THRESHOLD;
> --
> 2.8.2.311.gee88674
^ permalink raw reply
* Re: [PATCH] kbuild: Steal gcc's pie from the very beginning
From: Michal Marek @ 2016-11-14 20:45 UTC (permalink / raw)
To: Borislav Petkov, linux-kbuild; +Cc: Ben Hutchings, Sebastian Andrzej Siewior
In-Reply-To: <20161114185130.30641-1-bp@alien8.de>
Dne 14.11.2016 v 19:51 Borislav Petkov napsal(a):
> From: Borislav Petkov <bp@suse.de>
>
> So Sebastian turned off the PIE for kernel builds but that was too late
> - Kbuild.include already uses KBUILD_CFLAGS and trying to disable gcc
> options with, say cc-disable-warning, fails:
>
> gcc -D__KERNEL__ -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs
> ...
> -Wno-sign-compare -fno-asynchronous-unwind-tables -Wframe-address -c -x c /dev/null -o .31392.tmp
> /dev/null:1:0: error: code model kernel does not support PIC mode
>
> because that returns an error and we can't disable the warning. For
> example in this case:
>
> KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
>
> which leads to gcc issuing all those warnings again.
>
> So let's turn off PIE/PIC at the earliest possible moment, when we
> declare KBUILD_CFLAGS so that cc-disable-warning picks it up too.
>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: Ben Hutchings <ben@decadent.org.uk>
> Cc: Michal Marek <mmarek@suse.com>
> Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> Makefile | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 3ff1fb26d8b2..5558cc5b1505 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -399,11 +399,12 @@ KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> -fno-strict-aliasing -fno-common \
> -Werror-implicit-function-declaration \
> -Wno-format-security \
> - -std=gnu89
> + -std=gnu89 -fno-PIE
Please use the cc-option variant, otherwise good catch.
Michal
^ permalink raw reply
* [Buildroot] [PATCH] toolchain: Bump ARC tools to arc-2016.09-rc1
From: Thomas Petazzoni @ 2016-11-14 20:45 UTC (permalink / raw)
To: buildroot
In-Reply-To: <1479146197-41383-1-git-send-email-vzakhar@synopsys.com>
Hello,
On Mon, 14 Nov 2016 20:56:37 +0300, Vlad Zakharov wrote:
> As described at:
> 4520524ba055706236db9f00dd79f1b2e2e87fde
> this commit continues a series of updates of ARC tools.
> This time we're updating tools to arc-2016.09-rc1.
>
> This update contains a lot of important fixes, e.g. it fixes:
> http://autobuild.buildroot.net/results/4c7/4c77f33c842b37bf28cb931edf1b290e1bf4d93c//
> http://autobuild.buildroot.net/results/902/902729a0b98675ad803939e3ecdcf230065a6012//
> and other failures.
>
> Other important change is that we also update gdb. Now we are
> using gdb 7.12.
>
> This version of gdb requires C++ toolchain support so we add
> corresponding dependency to gdb Config.in file.
>
> Signed-off-by: Vlad Zakharov <vzakhar@synopsys.com>
Thanks. Even though it's a bump of a major component, it's fixing some
annoying build issues, and I guess a "rc" release is better than the
"engineering builds" we were using so far, so I've applied to master.
However, I had two things to fix before applying, see below.
> + # Since ARC gdb moved to 7.12 toolchain requires C++ support to build gdb.
> + depends on !BR2_arc || BR2_TOOLCHAIN_BUILDROOT_CXX
BR2_TOOLCHAIN_BUILDROOT_CXX is not correct here. This symbol only makes
sense in the context of the internal toolchain backend. But the target
gdb package makes perfect sense even when an external toolchain is
used.
To add a dependency on C++ support, you must use the
BR2_INSTALL_LIBSTDCPP symbol. This one is valid regardless of whether
you're using the internal toolchain backend or the external toolchain
backend.
Also, you forgot to add a comment about this C++ dependency, so I've
added it.
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH -v1.1] kbuild: Steal gcc's pie from the very beginning
From: Borislav Petkov @ 2016-11-14 20:45 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: linux-kbuild, Ben Hutchings, Michal Marek, x86-ml, lkml, +
In-Reply-To: <20161114201457.qsevfvn465otwfvz@linutronix.de>
On Mon, Nov 14, 2016 at 09:14:57PM +0100, Sebastian Andrzej Siewior wrote:
> This also means that all feature tests fail unless the -fno-PIE is
> there.
>
> Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>
> This thing gcc v3.4 only but I intend to update the min requirement to
> alteast gcc v3.4 to cover this. As per hpa gcc v3.4 is the lowest
> version should be used for x86…
Ah yes, here's a v1.1.
Let's leave the cc-option check until we sort out the min supported
gcc version.
CC: stable too.
Thanks!
---
From: Borislav Petkov <bp@suse.de>
Date: Mon, 14 Nov 2016 19:41:31 +0100
Subject: [PATCH] kbuild: Steal gcc's pie from the very beginning
So Sebastian turned off the PIE for kernel builds but that was too late
- Kbuild.include already uses KBUILD_CFLAGS and trying to disable gcc
options with, say cc-disable-warning, fails:
gcc -D__KERNEL__ -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs
...
-Wno-sign-compare -fno-asynchronous-unwind-tables -Wframe-address -c -x c /dev/null -o .31392.tmp
/dev/null:1:0: error: code model kernel does not support PIC mode
because that returns an error and we can't disable the warning. For
example in this case:
KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
which leads to gcc issuing all those warnings again.
So let's turn off PIE/PIC at the earliest possible moment, when we
declare KBUILD_CFLAGS so that cc-disable-warning picks it up too.
Also, we need the $(call cc-option ...) because -fno-PIE is supported
since gcc v3.4 and our lowest supported gcc version is 3.2 right now.
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: stable@vger.kernel.org
Cc: Ben Hutchings <ben@decadent.org.uk>
Cc: Michal Marek <mmarek@suse.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
Makefile | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index 3ff1fb26d8b2..9bc877d073d7 100644
--- a/Makefile
+++ b/Makefile
@@ -399,11 +399,12 @@ KBUILD_CFLAGS := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
-fno-strict-aliasing -fno-common \
-Werror-implicit-function-declaration \
-Wno-format-security \
- -std=gnu89
+ -std=gnu89 $(call cc-option,-fno-PIE)
+
KBUILD_AFLAGS_KERNEL :=
KBUILD_CFLAGS_KERNEL :=
-KBUILD_AFLAGS := -D__ASSEMBLY__
+KBUILD_AFLAGS := -D__ASSEMBLY__ $(call cc-option,-fno-PIE)
KBUILD_AFLAGS_MODULE := -DMODULE
KBUILD_CFLAGS_MODULE := -DMODULE
KBUILD_LDFLAGS_MODULE := -T $(srctree)/scripts/module-common.lds
@@ -621,8 +622,6 @@ include arch/$(SRCARCH)/Makefile
KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)
KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
-KBUILD_CFLAGS += $(call cc-option,-fno-PIE)
-KBUILD_AFLAGS += $(call cc-option,-fno-PIE)
ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
KBUILD_CFLAGS += $(call cc-option,-ffunction-sections,)
--
2.10.0
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply related
* Re: [PATCH 1/5] NFSv4: Don't check file access when reclaiming state
From: Anna Schumaker @ 2016-11-14 20:44 UTC (permalink / raw)
To: Trond Myklebust, linux-nfs
In-Reply-To: <1478814070-17140-2-git-send-email-trond.myklebust@primarydata.com>
Hi Trond,
On 11/10/2016 04:41 PM, Trond Myklebust wrote:
> If we're reclaiming state after a reboot, or as part of returning a
> delegation, we don't need to check access modes again.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> ---
> fs/nfs/nfs4proc.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 6a1d650e0419..4eead738da8e 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -1221,6 +1221,7 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
> atomic_inc(&sp->so_count);
> p->o_arg.open_flags = flags;
> p->o_arg.fmode = fmode & (FMODE_READ|FMODE_WRITE);
> + p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim);
> p->o_arg.share_access = nfs4_map_atomic_open_share(server,
> fmode, flags);
> /* don't put an ACCESS op in OPEN compound if O_EXCL, because ACCESS
> @@ -1228,8 +1229,14 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
> if (!(flags & O_EXCL)) {
> /* ask server to check for all possible rights as results
> * are cached */
> - p->o_arg.access = NFS4_ACCESS_READ | NFS4_ACCESS_MODIFY |
> - NFS4_ACCESS_EXTEND | NFS4_ACCESS_EXECUTE;
> + switch (p->o_arg.claim) {
> + case NFS4_OPEN_CLAIM_NULL:
> + case NFS4_OPEN_CLAIM_FH:
> + p->o_arg.access = NFS4_ACCESS_READ |
> + NFS4_ACCESS_MODIFY |
> + NFS4_ACCESS_EXTEND |
> + NFS4_ACCESS_EXECUTE;
> + }
This adds these warnings when I compile:
fs/nfs/nfs4proc.c: In function 'nfs4_opendata_alloc':
fs/nfs/nfs4proc.c:1232:3: error: enumeration value 'NFS4_OPEN_CLAIM_PREVIOUS' not handled in switch [-Werror=switch]
switch (p->o_arg.claim) {
^~~~~~
fs/nfs/nfs4proc.c:1232:3: error: enumeration value 'NFS4_OPEN_CLAIM_DELEGATE_CUR' not handled in switch [-Werror=switch]
fs/nfs/nfs4proc.c:1232:3: error: enumeration value 'NFS4_OPEN_CLAIM_DELEGATE_PREV' not handled in switch [-Werror=switch]
fs/nfs/nfs4proc.c:1232:3: error: enumeration value 'NFS4_OPEN_CLAIM_DELEG_CUR_FH' not handled in switch [-Werror=switch]
fs/nfs/nfs4proc.c:1232:3: error: enumeration value 'NFS4_OPEN_CLAIM_DELEG_PREV_FH' not handled in switch [-Werror=switch]
Thanks,
Anna
> }
> p->o_arg.clientid = server->nfs_client->cl_clientid;
> p->o_arg.id.create_time = ktime_to_ns(sp->so_seqid.create_time);
> @@ -1239,7 +1246,6 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
> p->o_arg.bitmask = nfs4_bitmask(server, label);
> p->o_arg.open_bitmap = &nfs4_fattr_bitmap[0];
> p->o_arg.label = nfs4_label_copy(p->a_label, label);
> - p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim);
> switch (p->o_arg.claim) {
> case NFS4_OPEN_CLAIM_NULL:
> case NFS4_OPEN_CLAIM_DELEGATE_CUR:
>
^ permalink raw reply
* Re: [PATCH 2/4] proc: Pass file mode to proc_pid_make_inode
From: Paul Moore @ 2016-11-14 20:44 UTC (permalink / raw)
To: Andreas Gruenbacher; +Cc: Stephen Smalley, Eric Paris, selinux
In-Reply-To: <1478812710-17190-3-git-send-email-agruenba@redhat.com>
On Thu, Nov 10, 2016 at 4:18 PM, Andreas Gruenbacher
<agruenba@redhat.com> wrote:
> Pass the file mode of the proc inode to be created to
> proc_pid_make_inode. In proc_pid_make_inode, initialize inode->i_mode
> before calling security_task_to_inode. This allows selinux to set
> isec->sclass right away without introducing "half-initialized" inode
> security structs.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
> fs/proc/base.c | 23 +++++++++--------------
> fs/proc/fd.c | 6 ++----
> fs/proc/internal.h | 2 +-
> fs/proc/namespaces.c | 3 +--
> security/selinux/hooks.c | 1 +
> 5 files changed, 14 insertions(+), 21 deletions(-)
Seems reasonable, merged.
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index ca651ac..6eae4d0 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -1664,7 +1664,8 @@ const struct inode_operations proc_pid_link_inode_operations = {
>
> /* building an inode */
>
> -struct inode *proc_pid_make_inode(struct super_block * sb, struct task_struct *task)
> +struct inode *proc_pid_make_inode(struct super_block * sb,
> + struct task_struct *task, umode_t mode)
> {
> struct inode * inode;
> struct proc_inode *ei;
> @@ -1678,6 +1679,7 @@ struct inode *proc_pid_make_inode(struct super_block * sb, struct task_struct *t
>
> /* Common stuff */
> ei = PROC_I(inode);
> + inode->i_mode = mode;
> inode->i_ino = get_next_ino();
> inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
> inode->i_op = &proc_def_inode_operations;
> @@ -2004,7 +2006,9 @@ proc_map_files_instantiate(struct inode *dir, struct dentry *dentry,
> struct proc_inode *ei;
> struct inode *inode;
>
> - inode = proc_pid_make_inode(dir->i_sb, task);
> + inode = proc_pid_make_inode(dir->i_sb, task, S_IFLNK |
> + ((mode & FMODE_READ ) ? S_IRUSR : 0) |
> + ((mode & FMODE_WRITE) ? S_IWUSR : 0));
> if (!inode)
> return -ENOENT;
>
> @@ -2013,12 +2017,6 @@ proc_map_files_instantiate(struct inode *dir, struct dentry *dentry,
>
> inode->i_op = &proc_map_files_link_inode_operations;
> inode->i_size = 64;
> - inode->i_mode = S_IFLNK;
> -
> - if (mode & FMODE_READ)
> - inode->i_mode |= S_IRUSR;
> - if (mode & FMODE_WRITE)
> - inode->i_mode |= S_IWUSR;
>
> d_set_d_op(dentry, &tid_map_files_dentry_operations);
> d_add(dentry, inode);
> @@ -2372,12 +2370,11 @@ static int proc_pident_instantiate(struct inode *dir,
> struct inode *inode;
> struct proc_inode *ei;
>
> - inode = proc_pid_make_inode(dir->i_sb, task);
> + inode = proc_pid_make_inode(dir->i_sb, task, p->mode);
> if (!inode)
> goto out;
>
> ei = PROC_I(inode);
> - inode->i_mode = p->mode;
> if (S_ISDIR(inode->i_mode))
> set_nlink(inode, 2); /* Use getattr to fix if necessary */
> if (p->iop)
> @@ -3059,11 +3056,10 @@ static int proc_pid_instantiate(struct inode *dir,
> {
> struct inode *inode;
>
> - inode = proc_pid_make_inode(dir->i_sb, task);
> + inode = proc_pid_make_inode(dir->i_sb, task, S_IFDIR | S_IRUGO | S_IXUGO);
> if (!inode)
> goto out;
>
> - inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
> inode->i_op = &proc_tgid_base_inode_operations;
> inode->i_fop = &proc_tgid_base_operations;
> inode->i_flags|=S_IMMUTABLE;
> @@ -3352,11 +3348,10 @@ static int proc_task_instantiate(struct inode *dir,
> struct dentry *dentry, struct task_struct *task, const void *ptr)
> {
> struct inode *inode;
> - inode = proc_pid_make_inode(dir->i_sb, task);
> + inode = proc_pid_make_inode(dir->i_sb, task, S_IFDIR | S_IRUGO | S_IXUGO);
>
> if (!inode)
> goto out;
> - inode->i_mode = S_IFDIR|S_IRUGO|S_IXUGO;
> inode->i_op = &proc_tid_base_inode_operations;
> inode->i_fop = &proc_tid_base_operations;
> inode->i_flags|=S_IMMUTABLE;
> diff --git a/fs/proc/fd.c b/fs/proc/fd.c
> index d21dafe..4274f83 100644
> --- a/fs/proc/fd.c
> +++ b/fs/proc/fd.c
> @@ -183,14 +183,13 @@ proc_fd_instantiate(struct inode *dir, struct dentry *dentry,
> struct proc_inode *ei;
> struct inode *inode;
>
> - inode = proc_pid_make_inode(dir->i_sb, task);
> + inode = proc_pid_make_inode(dir->i_sb, task, S_IFLNK);
> if (!inode)
> goto out;
>
> ei = PROC_I(inode);
> ei->fd = fd;
>
> - inode->i_mode = S_IFLNK;
> inode->i_op = &proc_pid_link_inode_operations;
> inode->i_size = 64;
>
> @@ -322,14 +321,13 @@ proc_fdinfo_instantiate(struct inode *dir, struct dentry *dentry,
> struct proc_inode *ei;
> struct inode *inode;
>
> - inode = proc_pid_make_inode(dir->i_sb, task);
> + inode = proc_pid_make_inode(dir->i_sb, task, S_IFREG | S_IRUSR);
> if (!inode)
> goto out;
>
> ei = PROC_I(inode);
> ei->fd = fd;
>
> - inode->i_mode = S_IFREG | S_IRUSR;
> inode->i_fop = &proc_fdinfo_file_operations;
>
> d_set_d_op(dentry, &tid_fd_dentry_operations);
> diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> index 5378441..f4494dc 100644
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -162,7 +162,7 @@ extern int proc_pid_statm(struct seq_file *, struct pid_namespace *,
> extern const struct dentry_operations pid_dentry_operations;
> extern int pid_getattr(struct vfsmount *, struct dentry *, struct kstat *);
> extern int proc_setattr(struct dentry *, struct iattr *);
> -extern struct inode *proc_pid_make_inode(struct super_block *, struct task_struct *);
> +extern struct inode *proc_pid_make_inode(struct super_block *, struct task_struct *, umode_t);
> extern int pid_revalidate(struct dentry *, unsigned int);
> extern int pid_delete_dentry(const struct dentry *);
> extern int proc_pid_readdir(struct file *, struct dir_context *);
> diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c
> index 51b8b0a..766f0c6 100644
> --- a/fs/proc/namespaces.c
> +++ b/fs/proc/namespaces.c
> @@ -92,12 +92,11 @@ static int proc_ns_instantiate(struct inode *dir,
> struct inode *inode;
> struct proc_inode *ei;
>
> - inode = proc_pid_make_inode(dir->i_sb, task);
> + inode = proc_pid_make_inode(dir->i_sb, task, S_IFLNK | S_IRWXUGO);
> if (!inode)
> goto out;
>
> ei = PROC_I(inode);
> - inode->i_mode = S_IFLNK|S_IRWXUGO;
> inode->i_op = &proc_ns_link_inode_operations;
> ei->ns_ops = ns_ops;
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index b98ab2a..e4527d9 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3954,6 +3954,7 @@ static void selinux_task_to_inode(struct task_struct *p,
> struct inode_security_struct *isec = inode->i_security;
> u32 sid = task_sid(p);
>
> + isec->sclass = inode_mode_to_security_class(inode->i_mode);
> isec->sid = sid;
> isec->initialized = LABEL_INITIALIZED;
> }
> --
> 2.7.4
>
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [U-Boot] [PATCH v3 7/8] x86: efi: Add a hello world test program
From: Simon Glass @ 2016-11-14 20:44 UTC (permalink / raw)
To: u-boot
In-Reply-To: <3D636533-5BE9-4029-BB71-ECB0C7605D12@suse.de>
Hi Alex,
On 11 November 2016 at 23:23, Alexander Graf <agraf@suse.de> wrote:
>
>
>> Am 11.11.2016 um 17:17 schrieb Simon Glass <sjg@chromium.org>:
>>
>> Hi Alex,
>>
>>> On 7 November 2016 at 09:32, Alexander Graf <agraf@suse.de> wrote:
>>>
>>>
>>>> On 07/11/2016 10:46, Simon Glass wrote:
>>>>
>>>> Hi Alex,
>>>>
>>>>> On 19 October 2016 at 01:09, Alexander Graf <agraf@suse.de> wrote:
>>>>>
>>>>>
>>>>>
>>>>>> On 18/10/2016 22:37, Simon Glass wrote:
>>>>>>
>>>>>> Hi Alex,
>>>>>>
>>>>>>> On 18 October 2016 at 01:14, Alexander Graf <agraf@suse.de> wrote:
>>>>>>>
>>>>>>>> On 10/18/2016 04:29 AM, Simon Glass wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> It is useful to have a basic sanity check for EFI loader support. Add
>>>>>>>> a
>>>>>>>> 'bootefi hello' command which loads HelloWord.efi and runs it under
>>>>>>>> U-Boot.
>>>>>>>>
>>>>>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Changes in v3:
>>>>>>>> - Include a link to the program instead of adding it to the tree
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> So, uh, where is the link?
>>>>>>
>>>>>>
>>>>>> I put it in the README (see the arm patch).
>>>>>>
>>>>>>>
>>>>>>> I'm really not convinced this command buys us anything yet. I do agree
>>>>>>> that
>>>>>>> we want automated testing - but can't we get that using QEMU and a
>>>>>>> downloadable image file that we pass in as disk and have the distro
>>>>>>> boot do
>>>>>>> its magic?
>>>>>>
>>>>>>
>>>>>> That seems very heavyweight as a sanity check, although I agree it is
>>>>>> useful.
>>>>>
>>>>>
>>>>> It's not really much more heavy weight. The "image file" could simply
>>>>> contain your hello world binary. But with this we don't just verify
>>>>> whether "bootefi" works, but also whether the default boot path works ok.
>>>>
>>>>
>>>> I don't think I understand what you mean by 'image file'. Is it
>>>> something other than the .efi file? Do you mean a disk image?
>>>
>>>
>>> Yes. For reasonable test coverage, we should also verify that the distro
>>> defaults wrote a sane boot script that automatically searches for a default
>>> EFI binary in /efi/boot/bootx86.efi on the first partition of all devices
>>> and runs it.
>>>
>>> So if we just provide an SD card image or hard disk image to QEMU which
>>> contains a hello world .efi binary as that default boot file, we don't only
>>> test whether the "bootefi" command works, but also whether the distro boot
>>> script works.
>>
>> That's right.
>>
>>>
>>>>
>>>>>
>>>>>> Here I am just making sure that EFI programs can start, print output
>>>>>> and exit. It is a test that we can easily run without a lot of
>>>>>> overhead, much less than a full distro boot.
>>>>>
>>>>>
>>>>> Again, I don't think it's much more overhead and I do believe it gives
>>>>> us much cleaner separation between responsibilities of code (tests go
>>>>> where tests are).
>>>>
>>>>
>>>> You are talking about a functional test, something that tests things
>>>> end to end. I prefer to at least start with a smaller test. Granted it
>>>> takes a little more work but it means there are fewer things to hunt
>>>> through when something goes wrong.
>>>
>>>
>>> Yes, I personally find unit tests terribly annoying and unproductive and
>>> functional tests very helpful :). And in this case, the effort to write it
>>> is about the same for both, just that the functional test actually tells you
>>> that things work or don't work at the end of the day.
>>>
>>> With a code base like U-Boot, a simple functional test like the above plus
>>> git bisect should get you to an offending patch very quickly.
>>
>> This is not a unit test - in fact the EFI stuff has no unit tests. I
>> suppose if we are trying to find a name this is a small functional
>> test since it exercises the general functionality.
>>
>> I am much keener on small tests than large ones for finding simple
>> bugs. Of course you can generally bisect to find a bug, but the more
>> layers of software you need to look for the harder this is.
>>
>> We could definitely use a pytest which checks an EFI boot into an
>> image, but I don't think this obviates the need for a smaller targeted
>> test like this one.
>
> I think arguing over this is moot :). More tests is usually a good thing, so whoever gets to write them gets to push them ;). As long as the licenses are sound at least.
OK good, well please can you review this at some point? Also, are you
planning to write the 'larger' test? How do you test this all in suse?
Regards,
Simon
^ permalink raw reply
* [U-Boot] [PATCH RESEND 7/9] video: Allow board hook before video init
From: Simon Glass @ 2016-11-14 20:44 UTC (permalink / raw)
To: u-boot
In-Reply-To: <20161114202407.jngcxs3rlm5d3u65@lukather>
Hi Maxime,
On 14 November 2016 at 13:24, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Simon,
>
> On Fri, Nov 11, 2016 at 09:17:28AM -0700, Simon Glass wrote:
>> Hi Maxime,
>>
>> On 8 November 2016 at 03:19, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > Some boards might need to some additional setup right before initialising
>> > the video console.
>> >
>> > Add some hook to allow that.
>>
>> Instead of this, can you use driver model (UCLASS_VIDEO)?
>
> I don't really know the device model that well, hence 'm not really
> sure how would that help. Can a board register a hook to be called
> before a driver is probed?
My suggest would be that the driver can do whatever is required. What
is the board-specific code actually wanting to do?
Regards,
Simon
^ permalink raw reply
* [U-Boot] [PATCH RESEND 6/9] eeprom: Add DS2431 support
From: Simon Glass @ 2016-11-14 20:44 UTC (permalink / raw)
To: u-boot
In-Reply-To: <20161114201238.peyv344yslxnkocj@lukather>
Hi Maxime,
On 14 November 2016 at 13:12, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Mon, Nov 14, 2016 at 10:14:57AM -0500, Tom Rini wrote:
>> On Mon, Nov 14, 2016 at 02:42:59PM +0100, Maxime Ripard wrote:
>> > Hi,
>> >
>> > On Fri, Nov 11, 2016 at 11:16:39AM -0800, Moritz Fischer wrote:
>> > > > +U_BOOT_DRIVER(ds2431) = {
>> > > > + .name = "ds2431",
>> > > > + .id = UCLASS_EEPROM,
>> > > > + .ops = &ds2431_ops,
>> > >
>> > > Do you want to add a .flags = DM_UC_FLAG_SEQ_ALIAS here?
>> >
>> > I don't know. I was kind of wondering why U-Boot relies on aliases so
>> > much, especially when the Linux DT maintainers are saying that aliases
>> > should be avoided entirely, and we'll won't be able to upstream those
>> > changes.
>>
>> Bah. Do you have a pointer to some discussion about this handy?
>
> Rob said this multiple times, but here is an example:
> http://lkml.iu.edu/hypermail/linux/kernel/1609.1/00653.html
Well that's not really an explanation!
- Simon
^ permalink raw reply
* [U-Boot] [PATCH RESEND 6/9] eeprom: Add DS2431 support
From: Simon Glass @ 2016-11-14 20:44 UTC (permalink / raw)
To: u-boot
In-Reply-To: <20161114134259.amnxlolu3urjrskb@lukather>
Hi Maxime,
On 14 November 2016 at 06:42, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Fri, Nov 11, 2016 at 11:16:39AM -0800, Moritz Fischer wrote:
>> > +U_BOOT_DRIVER(ds2431) = {
>> > + .name = "ds2431",
>> > + .id = UCLASS_EEPROM,
>> > + .ops = &ds2431_ops,
>>
>> Do you want to add a .flags = DM_UC_FLAG_SEQ_ALIAS here?
>
> I don't know. I was kind of wondering why U-Boot relies on aliases so
> much, especially when the Linux DT maintainers are saying that aliases
> should be avoided entirely, and we'll won't be able to upstream those
> changes.
U-Boot uses numbering on the command line for lots of device types.
E.g. the i2c bus number in the 'i2c' command. The aliases set the
numbering.
We should add support for moving away from numbering and using names,
at least as an option. I have not looked at that yet. Probably we
should consider changing command-line parsing to be handled in a
common library, with each command receiving a 'parsed' list of args
and options. I have not looked at that either.
Regards,
Simon
^ permalink raw reply
* [U-Boot] [RFC 1/1] image: Add TEE loading to FIT loadable processing
From: Simon Glass @ 2016-11-14 20:44 UTC (permalink / raw)
To: u-boot
In-Reply-To: <20161114194925.17117-2-afd@ti.com>
Hi Andrew,
On 14 November 2016 at 12:49, Andrew F. Davis <afd@ti.com> wrote:
> To help automate the loading of a TEE image during the boot we add a new
> FIT section type 'tee', when we see this type while loading the loadable
> sections we automatically call the platforms TEE processing function on
> this image section.
>
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> ---
> Kconfig | 10 ++++++++++
> common/image.c | 18 ++++++++++++++++++
> include/image.h | 15 +++++++++++++++
> 3 files changed, 43 insertions(+)
>
> diff --git a/Kconfig b/Kconfig
> index 1263d0b..97cf7c8 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -291,6 +291,16 @@ config FIT_IMAGE_POST_PROCESS
> injected into the FIT creation (i.e. the blobs would have been pre-
> processed before being added to the FIT image).
>
> +config FIT_IMAGE_TEE_PROCESS
> + bool "Enable processing of TEE images during FIT loading by U-Boot"
> + depends on FIT && TI_SECURE_DEVICE
This is a generic option so I don't think it should depend on TI.
> + help
> + Allows platforms to perform processing, such as authentication and
> + installation, on TEE images extracted from FIT images in a platform
> + or board specific way. In order to use this feature a platform or
> + board-specific implementation of board_tee_image_process() must be
> + provided.
> +
> config SPL_DFU_SUPPORT
> bool "Enable SPL with DFU to load binaries to memory device"
> depends on USB
> diff --git a/common/image.c b/common/image.c
> index 7604494..4552ca5 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -165,6 +165,7 @@ static const table_entry_t uimage_type[] = {
> { IH_TYPE_ZYNQIMAGE, "zynqimage", "Xilinx Zynq Boot Image" },
> { IH_TYPE_ZYNQMPIMAGE, "zynqmpimage", "Xilinx ZynqMP Boot Image" },
> { IH_TYPE_FPGA, "fpga", "FPGA Image" },
> + { IH_TYPE_TEE, "tee", "TEE OS Image",},
Perhaps write out TEE in full? It's a bit cryptic.
> { -1, "", "", },
> };
>
> @@ -1408,6 +1409,8 @@ int boot_get_loadable(int argc, char * const argv[], bootm_headers_t *images,
> int fit_img_result;
> const char *uname;
>
> + uint8_t img_type;
> +
> /* Check to see if the images struct has a FIT configuration */
> if (!genimg_has_config(images)) {
> debug("## FIT configuration was not specified\n");
> @@ -1447,6 +1450,21 @@ int boot_get_loadable(int argc, char * const argv[], bootm_headers_t *images,
> /* Something went wrong! */
> return fit_img_result;
> }
> +
> + fit_img_result = fit_image_get_node(buf, uname);
> + if (fit_img_result < 0) {
> + /* Something went wrong! */
> + return fit_img_result;
> + }
> + fit_img_result = fit_image_get_type(buf, fit_img_result, &img_type);
> + if (fit_img_result < 0) {
> + /* Something went wrong! */
> + return fit_img_result;
> + }
> +#if defined(CONFIG_FIT_IMAGE_TEE_PROCESS)
> + if (img_type == IH_TYPE_TEE)
> + board_tee_image_process(img_data, img_len);
> +#endif
Instead of putting this here, I think it would be better for
boot_get_loadable() to return the correct values for ld_start and
ld_len. Perhaps you need to pass it the loadable index to load, so it
is called multiple times? The only caller is bootm_find_images().
It is too ugly, I think, to check the image type in the 'load'
function, and do special things.
> }
> break;
> default:
> diff --git a/include/image.h b/include/image.h
> index 2b1296c..57084c8 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -279,6 +279,7 @@ enum {
> IH_TYPE_ZYNQMPIMAGE, /* Xilinx ZynqMP Boot Image */
> IH_TYPE_FPGA, /* FPGA Image */
> IH_TYPE_VYBRIDIMAGE, /* VYBRID .vyb Image */
> + IH_TYPE_TEE, /* Trusted Execution Environment OS Image */
>
> IH_TYPE_COUNT, /* Number of image types */
> };
> @@ -1263,4 +1264,18 @@ int board_fit_config_name_match(const char *name);
> void board_fit_image_post_process(void **p_image, size_t *p_size);
> #endif /* CONFIG_SPL_FIT_IMAGE_POST_PROCESS */
>
> +#ifdef CONFIG_FIT_IMAGE_TEE_PROCESS
I don't think you should have this #ifdef in the header file.
> +/**
> + * board_fit_tee_process() - Do any needed processing on a loaded TEE image
> + *
> + * This is used to verify, decrypt, and/or install a TEE in a platform or
> + * board specific way.
nit: board-specific
> + *
> + * @tee_image: pointer to the image
What format is the image?
> + * @tee_size: the image size
> + * @return no return value (failure should be handled internally)
> + */
> +void board_tee_image_process(void *tee_image, size_t tee_size);
I think it's a good idea to return an error code here, since the
function may fail.
> +#endif /* CONFIG_FIT_IMAGE_TEE_PROCESS */
> +
> #endif /* __IMAGE_H__ */
> --
> 2.10.1
>
Regards,
SImon
^ permalink raw reply
* [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE
From: Simon Glass @ 2016-11-14 20:44 UTC (permalink / raw)
To: u-boot
In-Reply-To: <20161114191419.14214-2-afd@ti.com>
Hi Andrew,
On 14 November 2016 at 12:14, Andrew F. Davis <afd@ti.com> wrote:
> Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE. An SPL which define
> this will abort image loading if the image is not a FIT image.
>
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> ---
> Kconfig | 9 +++++++++
> common/spl/spl.c | 5 +++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/Kconfig b/Kconfig
> index 1263d0b..eefebef 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -291,6 +291,15 @@ config FIT_IMAGE_POST_PROCESS
> injected into the FIT creation (i.e. the blobs would have been pre-
> processed before being added to the FIT image).
>
> +config SPL_ABORT_ON_NON_FIT_IMAGE
We already have CONFIG_IMAGE_FORMAT_LEGACY so how about
CONFIG_SPL_IMAGE_FORMAT_LEGACY instead? It can default to y if secure
boot is disabled.
> + bool "Disable SPL loading of non-FIT images"
> + default y if SPL_FIT_SIGNATURE
> + help
> + SPL will not load and image if it is not a FIT image. This is
> + useful for devices that only support authentication/encryption
> + through SPL FIT loading paths and do not want SPL falling back
> + to legacy image loading when a non-FIT image is present.
> +
> config SPL_DFU_SUPPORT
> bool "Enable SPL with DFU to load binaries to memory device"
> depends on USB
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index bdb165a..3d8bee9 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -93,6 +93,10 @@ void spl_set_header_raw_uboot(struct spl_image_info *spl_image)
> int spl_parse_image_header(struct spl_image_info *spl_image,
> const struct image_header *header)
> {
> +#ifdef CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE
> + /* non-FIT image found, proceed to other boot methods. */
> + return -EINVAL;
How about -EPROTONOSUPPORT since the request is not really invalid.
> +#else
> u32 header_size = sizeof(struct image_header);
>
> if (image_get_magic(header) == IH_MAGIC) {
> @@ -156,6 +160,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
> spl_set_header_raw_uboot(spl_image);
> #endif
> }
> +#endif
> return 0;
> }
>
> --
> 2.10.1
>
Regards,
Simon
^ permalink raw reply
* Re: [PATCH v2] cpufreq: conservative: Decrease frequency faster when the update deferred
From: Rafael J. Wysocki @ 2016-11-14 20:44 UTC (permalink / raw)
To: Stratos Karafotis
Cc: Rafael J. Wysocki, Viresh Kumar, linux-pm@vger.kernel.org, LKML
In-Reply-To: <691e286d-249b-e450-2df1-8421d83e6a46@semaphore.gr>
On Sat, Nov 12, 2016 at 10:04 PM, Stratos Karafotis
<stratosk@semaphore.gr> wrote:
> Conservative governor changes the CPU frequency in steps.
> That means that if a CPU runs at max frequency, it will need several
> sampling periods to return to min frequency when the workload
> is finished.
>
> If the update function that calculates the load and target frequency
> is deferred, the governor might need even more time to decrease the
> frequency.
>
> This may have impact to power consumption and after all conservative
> should decrease the frequency if there is no workload at every sampling
> rate.
>
> To resolve the above issue calculate the number of sampling periods
> that the update is deferred. Considering that for each sampling period
> conservative should drop the frequency by a freq_step because the
> CPU was idle apply the proper subtraction to requested frequency.
>
> Below, the kernel trace with and without this patch. First an
> intensive workload is applied on a specific CPU. Then the workload
> is removed and the CPU goes to idle.
>
> WITHOUT
>
> <idle>-0 [007] dN.. 620.329153: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-556 [007] .... 620.350857: cpu_frequency: state=1700000 cpu_id=7
> kworker/7:2-556 [007] .... 620.370856: cpu_frequency: state=1900000 cpu_id=7
> kworker/7:2-556 [007] .... 620.390854: cpu_frequency: state=2100000 cpu_id=7
> kworker/7:2-556 [007] .... 620.411853: cpu_frequency: state=2200000 cpu_id=7
> kworker/7:2-556 [007] .... 620.432854: cpu_frequency: state=2400000 cpu_id=7
> kworker/7:2-556 [007] .... 620.453854: cpu_frequency: state=2600000 cpu_id=7
> kworker/7:2-556 [007] .... 620.494856: cpu_frequency: state=2900000 cpu_id=7
> kworker/7:2-556 [007] .... 620.515856: cpu_frequency: state=3100000 cpu_id=7
> kworker/7:2-556 [007] .... 620.536858: cpu_frequency: state=3300000 cpu_id=7
> kworker/7:2-556 [007] .... 620.557857: cpu_frequency: state=3401000 cpu_id=7
> <idle>-0 [007] d... 669.591363: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 669.591939: cpu_idle: state=4294967295 cpu_id=7
> <idle>-0 [007] d... 669.591980: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] dN.. 669.591989: cpu_idle: state=4294967295 cpu_id=7
> ...
> <idle>-0 [007] d... 670.201224: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 670.221975: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-556 [007] .... 670.222016: cpu_frequency: state=3300000 cpu_id=7
> <idle>-0 [007] d... 670.222026: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 670.234964: cpu_idle: state=4294967295 cpu_id=7
> ...
> <idle>-0 [007] d... 670.801251: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 671.236046: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-556 [007] .... 671.236073: cpu_frequency: state=3100000 cpu_id=7
> <idle>-0 [007] d... 671.236112: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 671.393437: cpu_idle: state=4294967295 cpu_id=7
> ...
> <idle>-0 [007] d... 671.401277: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 671.404083: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-556 [007] .... 671.404111: cpu_frequency: state=2900000 cpu_id=7
> <idle>-0 [007] d... 671.404125: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 671.404974: cpu_idle: state=4294967295 cpu_id=7
> ...
> <idle>-0 [007] d... 671.501180: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 671.995414: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-556 [007] .... 671.995459: cpu_frequency: state=2800000 cpu_id=7
> <idle>-0 [007] d... 671.995469: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 671.996287: cpu_idle: state=4294967295 cpu_id=7
> ...
> <idle>-0 [007] d... 672.001305: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 672.078374: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-556 [007] .... 672.078410: cpu_frequency: state=2600000 cpu_id=7
> <idle>-0 [007] d... 672.078419: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 672.158020: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-556 [007] .... 672.158040: cpu_frequency: state=2400000 cpu_id=7
> <idle>-0 [007] d... 672.158044: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 672.160038: cpu_idle: state=4294967295 cpu_id=7
> ...
> <idle>-0 [007] d... 672.234557: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 672.237121: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-556 [007] .... 672.237174: cpu_frequency: state=2100000 cpu_id=7
> <idle>-0 [007] d... 672.237186: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 672.237778: cpu_idle: state=4294967295 cpu_id=7
> ...
> <idle>-0 [007] d... 672.267902: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 672.269860: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-556 [007] .... 672.269906: cpu_frequency: state=1900000 cpu_id=7
> <idle>-0 [007] d... 672.269914: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 672.271902: cpu_idle: state=4294967295 cpu_id=7
> ...
> <idle>-0 [007] d... 672.751342: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 672.823056: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-556 [007] .... 672.823095: cpu_frequency: state=1600000 cpu_id=7
>
> WITH
>
> <idle>-0 [007] dN.. 4380.928009: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-399 [007] .... 4380.949767: cpu_frequency: state=2000000 cpu_id=7
> kworker/7:2-399 [007] .... 4380.969765: cpu_frequency: state=2200000 cpu_id=7
> kworker/7:2-399 [007] .... 4381.009766: cpu_frequency: state=2500000 cpu_id=7
> kworker/7:2-399 [007] .... 4381.029767: cpu_frequency: state=2600000 cpu_id=7
> kworker/7:2-399 [007] .... 4381.049769: cpu_frequency: state=2800000 cpu_id=7
> kworker/7:2-399 [007] .... 4381.069769: cpu_frequency: state=3000000 cpu_id=7
> kworker/7:2-399 [007] .... 4381.089771: cpu_frequency: state=3100000 cpu_id=7
> kworker/7:2-399 [007] .... 4381.109772: cpu_frequency: state=3400000 cpu_id=7
> kworker/7:2-399 [007] .... 4381.129773: cpu_frequency: state=3401000 cpu_id=7
> <idle>-0 [007] d... 4428.226159: cpu_idle: state=1 cpu_id=7
> <idle>-0 [007] d... 4428.226176: cpu_idle: state=4294967295 cpu_id=7
> <idle>-0 [007] d... 4428.226181: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 4428.227177: cpu_idle: state=4294967295 cpu_id=7
> ...
> <idle>-0 [007] d... 4428.551640: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 4428.649239: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-399 [007] .... 4428.649268: cpu_frequency: state=2800000 cpu_id=7
> <idle>-0 [007] d... 4428.649278: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 4428.689856: cpu_idle: state=4294967295 cpu_id=7
> ...
> <idle>-0 [007] d... 4428.799542: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 4428.801683: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-399 [007] .... 4428.801748: cpu_frequency: state=1700000 cpu_id=7
> <idle>-0 [007] d... 4428.801761: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 4428.806545: cpu_idle: state=4294967295 cpu_id=7
> ...
> <idle>-0 [007] d... 4429.051880: cpu_idle: state=4 cpu_id=7
> <idle>-0 [007] d... 4429.086240: cpu_idle: state=4294967295 cpu_id=7
> kworker/7:2-399 [007] .... 4429.086293: cpu_frequency: state=1600000 cpu_id=7
>
> Without the patch the CPU dropped to min frequency after 3.2s
> With the patch applied the CPU dropped to min frequency after 0.86s
>
> Signed-off-by: Stratos Karafotis <stratosk@semaphore.gr>
> ---
> v1 -> v2
> - Use correct terminology in change log
> - Change the member variable name from 'deferred_periods' to 'idle_periods'
> - Fix format issue
>
> drivers/cpufreq/cpufreq_conservative.c | 14 +++++++++++++-
> drivers/cpufreq/cpufreq_governor.c | 18 +++++++++++++-----
> drivers/cpufreq/cpufreq_governor.h | 1 +
> 3 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index fa5ece3..d787772 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -73,7 +73,19 @@ static unsigned int cs_dbs_update(struct cpufreq_policy *policy)
> */
> if (cs_tuners->freq_step == 0)
> goto out;
> -
> + /*
> + * Decrease requested_freq for each idle period that we didn't
> + * update the frequency
> + */
> + if (policy_dbs->idle_periods < UINT_MAX) {
> + unsigned int freq_target = policy_dbs->idle_periods *
> + get_freq_target(cs_tuners, policy);
> + if (requested_freq > freq_target)
> + requested_freq -= freq_target;
> + else
> + requested_freq = policy->min;
> + policy_dbs->idle_periods = UINT_MAX;
> + }
> /*
> * If requested_freq is out of range, it is likely that the limits
> * changed in the meantime, so fall back to current frequency in that
> diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
> index 3729474..1bc7137 100644
> --- a/drivers/cpufreq/cpufreq_governor.c
> +++ b/drivers/cpufreq/cpufreq_governor.c
> @@ -117,7 +117,7 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
> struct policy_dbs_info *policy_dbs = policy->governor_data;
> struct dbs_data *dbs_data = policy_dbs->dbs_data;
> unsigned int ignore_nice = dbs_data->ignore_nice_load;
> - unsigned int max_load = 0;
> + unsigned int max_load = 0, idle_periods = UINT_MAX;
> unsigned int sampling_rate, io_busy, j;
>
> /*
> @@ -163,8 +163,12 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
> * calls, so the previous load value can be used then.
> */
> load = j_cdbs->prev_load;
> - } else if (unlikely(time_elapsed > 2 * sampling_rate &&
> - j_cdbs->prev_load)) {
> + } else if (unlikely(time_elapsed > 2 * sampling_rate)) {
> + unsigned int periods = time_elapsed / sampling_rate;
> +
> + if (periods < idle_periods)
> + idle_periods = periods;
> +
> /*
> * If the CPU had gone completely idle and a task has
> * just woken up on this CPU now, it would be unfair to
> @@ -189,8 +193,10 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
> * 'time_elapsed' (as compared to the sampling rate)
> * indicates this scenario.
> */
> - load = j_cdbs->prev_load;
> - j_cdbs->prev_load = 0;
> + if (j_cdbs->prev_load) {
> + load = j_cdbs->prev_load;
> + j_cdbs->prev_load = 0;
> + }
> } else {
> if (time_elapsed >= idle_time) {
> load = 100 * (time_elapsed - idle_time) / time_elapsed;
> @@ -218,6 +224,8 @@ unsigned int dbs_update(struct cpufreq_policy *policy)
> if (load > max_load)
> max_load = load;
> }
> + policy_dbs->idle_periods = idle_periods;
> +
> return max_load;
> }
> EXPORT_SYMBOL_GPL(dbs_update);
I have a murky suspicion that the changes in dbs_update() are going to
break something. I need to recall what it was, though.
Thanks,
Rafael
^ permalink raw reply
* [U-Boot] [PATCH 4/9] w1: Add 1-Wire gpio driver
From: Simon Glass @ 2016-11-14 20:44 UTC (permalink / raw)
To: u-boot
In-Reply-To: <20161114133536.brde45x2hdp2sjdo@lukather>
Hi Maxime,
On 14 November 2016 at 06:35, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Simon,
>
> On Fri, Nov 11, 2016 at 09:17:20AM -0700, Simon Glass wrote:
>> Hi Maxime,
>>
>> On 8 November 2016 at 03:06, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > Add a bus driver for bitbanging a 1-Wire bus over a GPIO.
>> >
>> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> > ---
>> > drivers/w1/Kconfig | 6 ++-
>> > drivers/w1/Makefile | 1 +-
>> > drivers/w1/w1-gpio.c | 160 ++++++++++++++++++++++++++++++++++++++++++++-
>> > 3 files changed, 167 insertions(+), 0 deletions(-)
>> > create mode 100644 drivers/w1/w1-gpio.c
>> >
>> > diff --git a/drivers/w1/Kconfig b/drivers/w1/Kconfig
>> > index 0c056b4c06a9..ccc3ae15db86 100644
>> > --- a/drivers/w1/Kconfig
>> > +++ b/drivers/w1/Kconfig
>> > @@ -12,6 +12,12 @@ config W1
>> >
>> > if W1
>> >
>> > +config W1_GPIO
>> > + bool "Enable 1-Wire GPIO bitbanging"
>> > + depends on DM_GPIO
>> > + help
>> > + Emulate a 1-Wire bus using a GPIO.
>>
>> Any more details? How many GPIOs? Any particular chips that are
>> supported?
>
> 1-Wire works on a single line, hence it's name. You usually have
> either a controller (which is quite rare, but some SoCs have one) or
> you can bitbang the bus. It's low bandwidth enough that it doesn't
> really matter.
>
> I'm not sure if it answers your question. I'll address your other
> comments.
OK thanks - please can you add a bit more detail into the help.
Regards,
Simon
^ permalink raw reply
* [U-Boot] [PATCH 4/4] rk3036: enable the vbus regulator when borad_init
From: Simon Glass @ 2016-11-14 20:44 UTC (permalink / raw)
To: u-boot
In-Reply-To: <58298CD5.8000608@rock-chips.com>
Hi Kever,
On 14 November 2016 at 03:07, Kever Yang <kever.yang@rock-chips.com> wrote:
> Hi Simon,
>
> On 11/12/2016 12:17 AM, Simon Glass wrote:
>>
>> Hi Kever,
>>
>> On 8 November 2016 at 03:13, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>
>>> enable the vbus for usb host in board_init().
>>
>> Note 'borad_init' typo in subject.
>
>
> Will fix in next version.
>>
>>
>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>> ---
>>>
>>> arch/arm/mach-rockchip/rk3036-board.c | 20 ++++++++++++++++++++
>>> 1 file changed, 20 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-rockchip/rk3036-board.c
>>> b/arch/arm/mach-rockchip/rk3036-board.c
>>> index bf2b268..90d3d33 100644
>>> --- a/arch/arm/mach-rockchip/rk3036-board.c
>>> +++ b/arch/arm/mach-rockchip/rk3036-board.c
>>> @@ -16,6 +16,7 @@
>>> #include <asm/arch/sdram_rk3036.h>
>>> #include <asm/gpio.h>
>>> #include <dm/pinctrl.h>
>>> +#include <power/regulator.h>
>>>
>>> DECLARE_GLOBAL_DATA_PTR;
>>>
>>> @@ -57,7 +58,26 @@ int board_late_init(void)
>>>
>>> int board_init(void)
>>> {
>>> + int ret;
>>> + struct udevice *regulator;
>>> +
>>> + ret = regulator_get_by_platname("vcc5v0_host", ®ulator);
>>
>> Can this be done in the USB driver? Then you might be able to use
>> device_get_supply_regulator().
>
>
> In dwc2 controller, there do have a bit for host power to control a signal
> named HOST_DRV_VBUS and init at dwc_otg_core_host_init(), but we do not
> using that controller signal, and using a GPIO instead, which may be
> different
> in different board, so we usually enable it in board file.
>
> Let me have a try if we can move it to USB driver.
In that case it should be mentioned in the device tree, so you can
perhaps use gpio-regulator.
>
>>
>> In fact it looks like board_usb_init() should move into a driver.
>
>
> We are not using board_usb_init() for usb host now, this function is only
> used
> for usb gadget/udc.
OK I see, that's fine.
BTW the merge window will open soon so please send any other patches
you have planned for the next release.
Regards,
Simon
^ permalink raw reply
* [U-Boot] [PATCHv2 03/15] dm: pci: remove pci_bus_to_hose(0) calling
From: Simon Glass @ 2016-11-14 20:44 UTC (permalink / raw)
To: u-boot
In-Reply-To: <DB6PR0401MB2549FCA4FB536383BD17EAC684BC0@DB6PR0401MB2549.eurprd04.prod.outlook.com>
Hi,
On 14 November 2016 at 00:22, Z.Q. Hou <zhiqiang.hou@nxp.com> wrote:
> Hi Simon,
>
> Thanks for your comments!
>
>> -----Original Message-----
>> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
>> Sent: 2016?11?12? 0:18
>> To: Z.Q. Hou <zhiqiang.hou@nxp.com>
>> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Albert ARIBAUD
>> <albert.u.boot@aribaud.net>; Prabhakar Kushwaha
>> <prabhakar.kushwaha@nxp.com>; Huan Wang-B18965
>> <alison.wang@freescale.com>; Sumit Garg <sumit.garg@nxp.com>; Ruchika
>> Gupta <ruchika.gupta@nxp.com>; Saksham Jain
>> <saksham.jain@nxp.freescale.com>; york sun <york.sun@nxp.com>; M.H. Lian
>> <minghuan.lian@nxp.com>; Bin Meng <bmeng.cn@gmail.com>; Mingkai Hu
>> <mingkai.hu@nxp.com>
>> Subject: Re: [PATCHv2 03/15] dm: pci: remove pci_bus_to_hose(0) calling
>>
>> Hi,
>>
>> On 10 November 2016 at 03:58, Zhiqiang Hou <Zhiqiang.Hou@nxp.com>
>> wrote:
>> > From: Minghuan Lian <Minghuan.Lian@nxp.com>
>> >
>> > There may be multiple PCIe controllers in a SoC.
>> > It is not correct that always calling pci_bus_to_hose(0) to get the
>> > first PCIe controller for the PCIe device connected other controllers.
>> > We just remove this calling because hose always point the correct PCIe
>> > controller.
>> >
>> > Signed-off-by: Minghuan Lian <Minghuan.Lian@nxp.com>
>> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>> > ---
>> > V2:
>> > - No change
>> >
>> > drivers/pci/pci_common.c | 10 ----------
>> > 1 file changed, 10 deletions(-)
>>
>> So is 'hose' always a root PCI controller now? If so, can you comment these in
>> the header? I'm a bit confused by this
>
> Yes, the patch 02 of this patchset statement that the function pci_bus_to_hose() will return the root pci controller.
> And will add the comment for the function.
>
>>
>> We should perhaps move the regions[] array into its own struct separate from
>> pci_controller.
>
> Could you let this patch set in first? Because I think this is a big task, and so far we have no effort to do that.
Yes - please add a comment as above, and a TODO for yourself.
Reviewed-by: Simon Glass <sjg@chromium.org>
>
>>
>> >
>> > diff --git a/drivers/pci/pci_common.c b/drivers/pci/pci_common.c index
>> > 1755914..448e814 100644
>> > --- a/drivers/pci/pci_common.c
>> > +++ b/drivers/pci/pci_common.c
>> > @@ -181,11 +181,6 @@ phys_addr_t pci_hose_bus_to_phys(struct
>> pci_controller *hose,
>> > return phys_addr;
>> > }
>> >
>> > -#ifdef CONFIG_DM_PCI
>> > - /* The root controller has the region information */
>> > - hose = pci_bus_to_hose(0);
>> > -#endif
>> > -
>> > /*
>> > * if PCI_REGION_MEM is set we do a two pass search with
>> preference
>> > * on matches that don't have PCI_REGION_SYS_MEMORY set
>> @@
>> > -248,11 +243,6 @@ pci_addr_t pci_hose_phys_to_bus(struct pci_controller
>> *hose,
>> > return bus_addr;
>> > }
>> >
>> > -#ifdef CONFIG_DM_PCI
>> > - /* The root controller has the region information */
>> > - hose = pci_bus_to_hose(0);
>> > -#endif
>> > -
>> > /*
>> > * if PCI_REGION_MEM is set we do a two pass search with
>> preference
>> > * on matches that don't have PCI_REGION_SYS_MEMORY set
>> > --
>> > 2.1.0.27.g96db324
>> >
>
> Thanks,
> Zhiqiang
Regards,
Simon
^ permalink raw reply
* [U-Boot] [PATCH] fastboot: simplify the Kconfig logic
From: Simon Glass @ 2016-11-14 20:44 UTC (permalink / raw)
To: u-boot
In-Reply-To: <1479072373-7307-1-git-send-email-yann.morin.1998@free.fr>
+Tom
On 13 November 2016 at 14:26, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Currently, the fastboot item in menuconfig is a comment followed by a
> boolean option withan empty prompt, followed by a menu:
>
> *** FASTBOOT ***
> [*]
> Fastboot support --->
>
> This is not "nice-looking" at all...
>
> Change the logic to make the boolean option a "menuconfig" rather than a
> mere "config", so that all dependent options gets groupped under a menu.
> The layout is now:
>
> *** FASTBOOT ***
> [*] Fastboot support --->
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Simon Glass <sjg@chromium.org>
> ---
> cmd/fastboot/Kconfig | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply
* [U-Boot] [PATCH] usb: check udev before dereferencing
From: Simon Glass @ 2016-11-14 20:44 UTC (permalink / raw)
To: u-boot
In-Reply-To: <a6fef258-c3ea-f06f-2f14-550ede9d8d3e@denx.de>
Hi,
On 12 November 2016 at 11:17, Marek Vasut <marex@denx.de> wrote:
> On 11/12/2016 07:10 PM, Anatolij Gustschin wrote:
>> On Sat, 12 Nov 2016 10:36:42 +0100
>> Marek Vasut marex at denx.de wrote:
>> ...
>>>> udev = dev_get_parent_priv(child);
>>>> + if (!udev)
>>>> + continue;
>>>
>>> I don't quite understand the problem from the patch description, but
>>> shouldn't all the return values from dev_get_parent_priv() be checked
>>> this way , not just these two ?
>>
>> The problem is that when dereferencing NULL udev we later access
>> some random address (e.g. when accessing dev->dev->parent in
>> usb_show_tree_graph()). dev->dev pointer is random DRAM data there,
>> when dereferencing it, data abort happens when random address
>> is outside of valid address range.
>
> I mean, I understand that udev can be NULL and we don't check it. But is
> udev == NULL an expected possibility ? And if so, when does such thing
> happen ?
>
>> Probably we should check elsewhere, at least where it might
>> return NULL.
>
> OK
>
>>>
>>> Why does dev_get_parent_priv() return NULL here ?
>>
>> it returns NULL because the dev->parent_priv is not allocated for
>> usb_mass_storage.lun0 device. I do not know the reason why.
>
> That's probably what needs to be fixed , no ?
Yes that seems wrong. There is a check immediately before this:
if (!device_active(child))
continue;
If the device is active, it must have been probed and so must have parent data.
See:
UCLASS_DRIVER(usb) = {
...
.per_child_auto_alloc_size = sizeof(struct usb_device),
Try 'dm tree' to see what the bad USB device is.
>
> Also, we should most likely check all the return values of
> dev_get_parent_priv() in cmd/usb.c, not just these two.
Regards,
Simon
^ 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.