* Re: [PATCH v2 net-next 0/3] ipv4: Hash-based multipath routing
From: Thomas Graf @ 2015-08-31 9:02 UTC (permalink / raw)
To: Tom Herbert
Cc: Peter N??rlund, David Miller, Linux Kernel Network Developers,
Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, linux-api, Roopa Prabhu, sfeldma,
Eric W. Biederman, Nicolas Dichtel, Jiri Benc
In-Reply-To: <CALx6S36wZ36jS9fmrFJZ3BddnpGpBJXph5HtOBYQ=ekatSKDNw@mail.gmail.com>
On 08/30/15 at 03:29pm, Tom Herbert wrote:
> On Sun, Aug 30, 2015 at 2:28 PM, Peter N??rlund <pch@ordbogen.com> wrote:
> > It would definitely be simpler, and it would be nice to just fetch the
> > hash directly from the NIC - and for link aggregation it would probably
> > be fine. But with L4, we always need to consider fragmented packets,
> > which might cause some packets of a flow to be routed differently - and
> > with ECMP, the ramifications of suddenly choosing another path for a
> > flow are worse than for link aggregation. The latency through the
> > different paths may differ enough to cause out-or-order packets and bad
> > TCP performance as a consequence. Both Cisco and Juniper routers
> > defaults to L3 for ECMP - exactly for that reason, I believe. RFC 2991
> > also points out that ports probably shouldn't be used as part of the
> > flow key with ECMP.
> >
> That's more reason why we need vendors to use IPv6 flow label instead
> of ports to do ECMP :-). In any case, if we're fragmenting TCP packets
> then we're already in a bad place performance-wise-- we really don't
> need to optimize for that case. Albeit, it would be nice if fragments
> of packet followed same path, but the would require devices to not do
> L4 hash over ports when MF is set-- I don't know if anyone does that
> (I have been meaning to add that to stack).
+1 for solving this at hash level. Being able to rely on the L4 HW
hash for multipath routing is very desirable. A simple MF bit ||
FO > 0 check with fall back to flow dissector to generate an L3 hash
in case the HW provided an L4 hash should be sufficient to address the
fragmentation concern.
Since performance is gone anyway, I'm not sure it's worth offloading
this behaviour to the HW.
^ permalink raw reply
* Re: [PATCH v8 28/55] [media] uapi/media.h: Fix entity namespace
From: Hans Verkuil @ 2015-08-31 11:17 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Laurent Pinchart, Sakari Ailus,
Hans Verkuil, linux-api
In-Reply-To: <df9a272928de3326c7139210a1c957d35197d86e.1440902901.git.mchehab@osg.samsung.com>
On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Now that interfaces got created, we need to fix the entity
> namespace.
>
> So, let's create a consistent new namespace and add backward
> compatibility macros to keep the old namespace preserved.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 14d32cdcdd47..88013d1a2c39 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -230,17 +230,17 @@ static void dvb_create_media_entity(struct dvb_device *dvbdev,
>
> switch (type) {
> case DVB_DEVICE_FRONTEND:
> - dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
> + dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMOD;
> dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> break;
> case DVB_DEVICE_DEMUX:
> - dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
> + dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMUX;
> dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> break;
> case DVB_DEVICE_CA:
> - dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
> + dvbdev->entity->type = MEDIA_ENT_T_DVB_CA;
> dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> break;
> @@ -439,7 +439,7 @@ EXPORT_SYMBOL(dvb_unregister_device);
> void dvb_create_media_graph(struct dvb_adapter *adap)
> {
> struct media_device *mdev = adap->mdev;
> - struct media_entity *entity, *tuner = NULL, *fe = NULL;
> + struct media_entity *entity, *tuner = NULL, *demod = NULL;
> struct media_entity *demux = NULL, *dvr = NULL, *ca = NULL;
> struct media_interface *intf;
>
> @@ -451,26 +451,26 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
> case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
> tuner = entity;
> break;
> - case MEDIA_ENT_T_DEVNODE_DVB_FE:
> - fe = entity;
> + case MEDIA_ENT_T_DVB_DEMOD:
> + demod = entity;
> break;
> - case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
> + case MEDIA_ENT_T_DVB_DEMUX:
> demux = entity;
> break;
> - case MEDIA_ENT_T_DEVNODE_DVB_DVR:
> + case MEDIA_ENT_T_DVB_TSOUT:
> dvr = entity;
> break;
> - case MEDIA_ENT_T_DEVNODE_DVB_CA:
> + case MEDIA_ENT_T_DVB_CA:
> ca = entity;
> break;
> }
> }
>
> - if (tuner && fe)
> - media_create_pad_link(tuner, 0, fe, 0, 0);
> + if (tuner && demod)
> + media_create_pad_link(tuner, 0, demod, 0, 0);
>
> - if (fe && demux)
> - media_create_pad_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
> + if (demod && demux)
> + media_create_pad_link(demod, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
>
> if (demux && dvr)
> media_create_pad_link(demux, 1, dvr, 0, MEDIA_LNK_FL_ENABLED);
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index aca828709bad..3bbda409353f 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -42,31 +42,71 @@ struct media_device_info {
>
> #define MEDIA_ENT_ID_FLAG_NEXT (1 << 31)
>
> +/*
> + * Base numbers for entity types
> + *
> + * Please notice that the huge gap of 16 bits for each base is overkill!
> + * 8 bits is more than enough to avoid starving entity types for each
> + * subsystem.
> + *
> + * However, It is kept this way just to avoid binary breakages with the
> + * namespace provided on legacy versions of this header.
> + */
> +#define MEDIA_ENT_T_DVB_BASE 0x00000001
I would change this to 0x00000000, see follow-up comment later for why.
> +#define MEDIA_ENT_T_V4L2_BASE 0x00010000
> +#define MEDIA_ENT_T_V4L2_SUBDEV_BASE 0x00020000
> +
> +/*
> + * V4L2 entities - Those are used for DMA (mmap/DMABUF) and
> + * read()/write() data I/O associated with the V4L2 devnodes.
> + */
> +#define MEDIA_ENT_T_V4L2_VIDEO (MEDIA_ENT_T_V4L2_BASE + 1)
> + /*
> + * Please notice that numbers between MEDIA_ENT_T_V4L2_BASE + 2 and
> + * MEDIA_ENT_T_V4L2_BASE + 4 can't be used, as those values used
> + * to be declared for FB, ALSA and DVB entities.
> + * As those values were never actually used in practice, we're just
> + * adding them as backward compatibility macros and keeping the
> + * numberspace clean here. This way, we avoid breaking compilation,
> + * in the case of having some userspace application using the old
> + * symbols.
> + */
> +#define MEDIA_ENT_T_V4L2_VBI (MEDIA_ENT_T_V4L2_BASE + 5)
> + /* for TX radio, as RX is done via either ALSA or wire */
> +#define MEDIA_ENT_T_V4L2_RADIO (MEDIA_ENT_T_V4L2_BASE + 6)
But TX is also done via either ALSA or wire. This shouldn't be needed.
> +#define MEDIA_ENT_T_V4L2_SWRADIO (MEDIA_ENT_T_V4L2_BASE + 7)
How about MEDIA_ENT_T_DVB_IO_* and MEDIA_ENT_T_V4L2_IO_* to indicate that
this entity deals with data I/O?
Or, perhaps even better, MEDIA_ENT_T_IO_DVB_ and MEDIA_ENT_T_IO_V4L2_.
Entities should do something, and just saying 'V4L2_VIDEO' doesn't really convey
that meaning. It is also very easy to confuse with INTF_T_V4L_* types. BTW, we
should decide whether V4L2 or V4L is used here (interfaces now use V4L, entities
V4L2). Since entities already use V4L2, I think the interface defines should
use V4L2 as well.
> +
> +/* V4L2 Sub-device entities */
> +#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 1)
> +#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 2)
> +#define MEDIA_ENT_T_V4L2_SUBDEV_LENS (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 3)
> + /* A converter of analogue video to its digital representation. */
> +#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 4)
> + /* Tuner entity is actually both V4L2 and DVB subdev */
> +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 5)
> +
> +/* DVB entities */
> +#define MEDIA_ENT_T_DVB_DEMOD (MEDIA_ENT_T_DVB_BASE)
After changing DVB_BASE to 0, change this to DVB_BASE + 1, and adjust the other DVB
entity types accordingly.
This keeps the base defines consistent (i.e. the lower 16 bits are always 0).
It surprised me when reading this patch, so I'm probably not the only one.
Regards,
Hans
> +#define MEDIA_ENT_T_DVB_DEMUX (MEDIA_ENT_T_DVB_BASE + 1)
> +#define MEDIA_ENT_T_DVB_TSOUT (MEDIA_ENT_T_DVB_BASE + 2)
> +#define MEDIA_ENT_T_DVB_CA (MEDIA_ENT_T_DVB_BASE + 3)
> +#define MEDIA_ENT_T_DVB_NET_DECAP (MEDIA_ENT_T_DVB_BASE + 4)
> +
> +/* Legacy symbols used to avoid userspace compilation breakages */
> #define MEDIA_ENT_TYPE_SHIFT 16
> #define MEDIA_ENT_TYPE_MASK 0x00ff0000
> #define MEDIA_ENT_SUBTYPE_MASK 0x0000ffff
>
> -#define MEDIA_ENT_T_DEVNODE (1 << MEDIA_ENT_TYPE_SHIFT)
> -#define MEDIA_ENT_T_DEVNODE_V4L (MEDIA_ENT_T_DEVNODE + 1)
> +#define MEDIA_ENT_T_DEVNODE MEDIA_ENT_T_V4L2_BASE
> +#define MEDIA_ENT_T_V4L2_SUBDEV MEDIA_ENT_T_V4L2_SUBDEV_BASE
> +
> +#define MEDIA_ENT_T_DEVNODE_V4L MEDIA_ENT_T_V4L2_VIDEO
> +
> #define MEDIA_ENT_T_DEVNODE_FB (MEDIA_ENT_T_DEVNODE + 2)
> #define MEDIA_ENT_T_DEVNODE_ALSA (MEDIA_ENT_T_DEVNODE + 3)
> -#define MEDIA_ENT_T_DEVNODE_DVB_FE (MEDIA_ENT_T_DEVNODE + 4)
> -#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX (MEDIA_ENT_T_DEVNODE + 5)
> -#define MEDIA_ENT_T_DEVNODE_DVB_DVR (MEDIA_ENT_T_DEVNODE + 6)
> -#define MEDIA_ENT_T_DEVNODE_DVB_CA (MEDIA_ENT_T_DEVNODE + 7)
> -#define MEDIA_ENT_T_DEVNODE_DVB_NET (MEDIA_ENT_T_DEVNODE + 8)
> +#define MEDIA_ENT_T_DEVNODE_DVB (MEDIA_ENT_T_DEVNODE + 4)
>
> -/* Legacy symbol. Use it to avoid userspace compilation breakages */
> -#define MEDIA_ENT_T_DEVNODE_DVB MEDIA_ENT_T_DEVNODE_DVB_FE
> -
> -#define MEDIA_ENT_T_V4L2_SUBDEV (2 << MEDIA_ENT_TYPE_SHIFT)
> -#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR (MEDIA_ENT_T_V4L2_SUBDEV + 1)
> -#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH (MEDIA_ENT_T_V4L2_SUBDEV + 2)
> -#define MEDIA_ENT_T_V4L2_SUBDEV_LENS (MEDIA_ENT_T_V4L2_SUBDEV + 3)
> -/* A converter of analogue video to its digital representation. */
> -#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER (MEDIA_ENT_T_V4L2_SUBDEV + 4)
> -
> -#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER (MEDIA_ENT_T_V4L2_SUBDEV + 5)
> +/* Entity types */
>
> #define MEDIA_ENT_FL_DEFAULT (1 << 0)
>
>
^ permalink raw reply
* Re: [PATCH v8 38/55] [media] v4l2-subdev: use MEDIA_ENT_T_UNKNOWN for new subdevs
From: Hans Verkuil @ 2015-08-31 11:43 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List
Cc: Mauro Carvalho Chehab, Hans Verkuil, Laurent Pinchart,
Sylwester Nawrocki, Prabhakar Lad, Lars-Peter Clausen,
Markus Elfring, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <662a3bb2bfb02f820f4dafa0033af2cf16d273c5.1440902901.git.mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Instead of abusing MEDIA_ENT_T_V4L2_SUBDEV, initialize
> new subdev entities as MEDIA_ENT_T_UNKNOWN.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 659507bce63f..134fe7510195 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -435,6 +435,12 @@ int __must_check media_device_register_entity(struct media_device *mdev,
> {
> int i;
>
> + if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN ||
> + entity->type == MEDIA_ENT_T_UNKNOWN)
> + dev_warn(mdev->dev,
> + "Entity type for entity %s was not initialized!\n",
> + entity->name);
> +
> /* Warn if we apparently re-register an entity */
> WARN_ON(entity->graph_obj.mdev != NULL);
> entity->graph_obj.mdev = mdev;
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 60da43772de9..b3bcc8253182 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -584,7 +584,7 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops)
> sd->host_priv = NULL;
> #if defined(CONFIG_MEDIA_CONTROLLER)
> sd->entity.name = sd->name;
> - sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV;
> + sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN;
> #endif
> }
> EXPORT_SYMBOL(v4l2_subdev_init);
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 3bbda409353f..44b84aae8b02 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -42,6 +42,14 @@ struct media_device_info {
>
> #define MEDIA_ENT_ID_FLAG_NEXT (1 << 31)
>
> +/* Used values for media_entity_desc::type */
> +
> +/*
> + * Initial value when an entity is created
> + * Drivers should change it to something useful
> + */
> +#define MEDIA_ENT_T_UNKNOWN 0x00000000
> +
> /*
> * Base numbers for entity types
> *
> @@ -77,6 +85,15 @@ struct media_device_info {
> #define MEDIA_ENT_T_V4L2_SWRADIO (MEDIA_ENT_T_V4L2_BASE + 7)
>
> /* V4L2 Sub-device entities */
> +
> + /*
> + * Subdevs are initialized with MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN,
> + * in order to preserve backward compatibility.
> + * Drivers should change to the proper subdev type before
> + * registering the entity.
> + */
> +#define MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN MEDIA_ENT_T_V4L2_SUBDEV_BASE
> +
> #define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 1)
> #define MEDIA_ENT_T_V4L2_SUBDEV_FLASH (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 2)
> #define MEDIA_ENT_T_V4L2_SUBDEV_LENS (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 3)
>
^ permalink raw reply
* Re: [PATCH v8 39/55] [media] media controller: get rid of entity subtype on Kernel
From: Hans Verkuil @ 2015-08-31 11:44 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List
Cc: Mauro Carvalho Chehab, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <8154aa42b993840dfde2d794e7e9e1f0c57c1e82.1440902901.git.mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Don't use anymore the type/subtype entity data/macros
> inside the Kernel.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index b0cfbc0dffc7..756e1960fd7f 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -185,16 +185,6 @@ struct media_intf_devnode {
> u32 minor;
> };
>
> -static inline u32 media_entity_type(struct media_entity *entity)
> -{
> - return entity->type & MEDIA_ENT_TYPE_MASK;
> -}
> -
> -static inline u32 media_entity_subtype(struct media_entity *entity)
> -{
> - return entity->type & MEDIA_ENT_SUBTYPE_MASK;
> -}
> -
> static inline u32 media_entity_id(struct media_entity *entity)
> {
> return entity->graph_obj.id;
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 44b84aae8b02..cd486fc25f1e 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -42,10 +42,8 @@ struct media_device_info {
>
> #define MEDIA_ENT_ID_FLAG_NEXT (1 << 31)
>
> -/* Used values for media_entity_desc::type */
> -
> /*
> - * Initial value when an entity is created
> + * Initial value to be used when a new entity is created
This change should be moved to patch 38.
> * Drivers should change it to something useful
> */
> #define MEDIA_ENT_T_UNKNOWN 0x00000000
>
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v8 40/55] [media] media.h: don't use legacy entity macros at Kernel
From: Hans Verkuil @ 2015-08-31 11:44 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List
Cc: Mauro Carvalho Chehab, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <720b750e2738f8c70535b01c9c3a3dddf044db69.1440902901.git.mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Put the legacy MEDIA_ENT_* macros under a #ifndef __KERNEL__,
> in order to be sure that none of those old symbols are used
> inside the Kernel.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index cd486fc25f1e..4186891e5e81 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -107,6 +107,7 @@ struct media_device_info {
> #define MEDIA_ENT_T_DVB_CA (MEDIA_ENT_T_DVB_BASE + 3)
> #define MEDIA_ENT_T_DVB_NET_DECAP (MEDIA_ENT_T_DVB_BASE + 4)
>
> +#ifndef __KERNEL__
> /* Legacy symbols used to avoid userspace compilation breakages */
> #define MEDIA_ENT_TYPE_SHIFT 16
> #define MEDIA_ENT_TYPE_MASK 0x00ff0000
> @@ -120,6 +121,7 @@ struct media_device_info {
> #define MEDIA_ENT_T_DEVNODE_FB (MEDIA_ENT_T_DEVNODE + 2)
> #define MEDIA_ENT_T_DEVNODE_ALSA (MEDIA_ENT_T_DEVNODE + 3)
> #define MEDIA_ENT_T_DEVNODE_DVB (MEDIA_ENT_T_DEVNODE + 4)
> +#endif
>
> /* Entity types */
>
>
^ permalink raw reply
* Re: [PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl
From: Hans Verkuil @ 2015-08-31 12:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Media Mailing List
Cc: Mauro Carvalho Chehab, linux-api
In-Reply-To: <ed72ef83c937fe6f665001eb9d6a54f25f253391.1440902901.git.mchehab@osg.samsung.com>
On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> Add a new ioctl that will report the entire topology on
> one go.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 756e1960fd7f..358a0c6b1f86 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -181,6 +181,8 @@ struct media_interface {
> */
> struct media_intf_devnode {
> struct media_interface intf;
> +
> + /* Should match the fields at media_v2_intf_devnode */
> u32 major;
> u32 minor;
> };
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 4186891e5e81..fa0b68e670b0 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -251,11 +251,94 @@ struct media_links_enum {
> #define MEDIA_INTF_T_ALSA_RAWMIDI (MEDIA_INTF_T_ALSA_BASE + 4)
> #define MEDIA_INTF_T_ALSA_HWDEP (MEDIA_INTF_T_ALSA_BASE + 5)
>
> -/* TBD: declare the structs needed for the new G_TOPOLOGY ioctl */
> +/*
> + * MC next gen API definitions
> + *
> + * NOTE: The declarations below are close to the MC RFC for the Media
> + * Controller, the next generation. Yet, there are a few adjustments
> + * to do, as we want to be able to have a functional API before
> + * the MC properties change. Those will be properly marked below.
> + * Please also notice that I removed "num_pads", "num_links",
> + * from the proposal, as a proper userspace application will likely
> + * use lists for pads/links, just as we intend todo in Kernelspace.
s/todo/to do/
> + * The API definition should be freed from fields that are bound to
> + * some specific data structure.
> + *
> + * FIXME: Currently, I opted to name the new types as "media_v2", as this
> + * won't cause any conflict with the Kernelspace namespace, nor with
> + * the previous kAPI media_*_desc namespace. This can be changed
> + * latter, before the adding this API upstream.
s/latter/later/ :-)
I think this comment belongs to the commit log and not in this header.
> + */
> +
> +
> +#define MEDIA_NEW_LNK_FL_ENABLED MEDIA_LNK_FL_ENABLED
> +#define MEDIA_NEW_LNK_FL_IMMUTABLE MEDIA_LNK_FL_IMMUTABLE
> +#define MEDIA_NEW_LNK_FL_DYNAMIC MEDIA_NEW_FL_DYNAMIC
> +#define MEDIA_NEW_LNK_FL_INTERFACE_LINK (1 << 3)
Shouldn't this be MEDIA_V2_ instead of MEDIA_NEW_?
Do we need the INTERFACE_LINK flag? You can deduce it by checking the
ID type.
I don't have a clear preference one way or another, just wondering about the
reason for adding it.
> +
> +struct media_v2_entity {
> + __u32 id;
> + char name[64]; /* FIXME: move to a property? (RFC says so) */
> + __u16 reserved[14];
> +};
> +
> +/* Should match the specific fields at media_intf_devnode */
> +struct media_v2_intf_devnode {
> + __u32 major;
> + __u32 minor;
> +};
> +
> +struct media_v2_interface {
> + __u32 id;
> + __u32 intf_type;
> + __u32 flags;
> + __u32 reserved[9];
> +
> + union {
> + struct media_v2_intf_devnode devnode;
> + __u32 raw[16];
> + };
> +};
> +
> +struct media_v2_pad {
> + __u32 id;
> + __u32 entity_id;
> + __u32 flags;
> + __u16 reserved[9];
> +};
> +
> +struct media_v2_link {
> + __u32 id;
> + __u32 source_id;
> + __u32 sink_id;
Like in media_link I would use a union here as well to be able to refer to
source/sink_id and entity/interface_id.
> + __u32 flags;
> + __u32 reserved[5];
> +};
> +
> +struct media_v2_topology {
> + __u32 topology_version;
> +
> + __u32 num_entities;
> + struct media_v2_entity *entities;
> +
> + __u32 num_interfaces;
> + struct media_v2_interface *interfaces;
> +
> + __u32 num_pads;
> + struct media_v2_pad *pads;
> +
> + __u32 num_links;
> + struct media_v2_link *links;
> +
> + __u32 reserved[64];
As mentioned before: use this instead to prevent horrible 32/64 bit arch
compat code:
struct {
__u32 reserved_num;
void *reserved_ptr;
} reserved_types[16];
__u32 reserved[8];
Sizes for these arrays are TBD.
> +};
> +
> +/* ioctls */
>
> #define MEDIA_IOC_DEVICE_INFO _IOWR('|', 0x00, struct media_device_info)
> #define MEDIA_IOC_ENUM_ENTITIES _IOWR('|', 0x01, struct media_entity_desc)
> #define MEDIA_IOC_ENUM_LINKS _IOWR('|', 0x02, struct media_links_enum)
> #define MEDIA_IOC_SETUP_LINK _IOWR('|', 0x03, struct media_link_desc)
> +#define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct media_v2_topology)
>
> #endif /* __LINUX_MEDIA_H */
>
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v8 28/55] [media] uapi/media.h: Fix entity namespace
From: Mauro Carvalho Chehab @ 2015-08-31 12:12 UTC (permalink / raw)
To: Hans Verkuil
Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Laurent Pinchart,
Sakari Ailus, Hans Verkuil, linux-api
In-Reply-To: <55E437B4.2030106@xs4all.nl>
Em Mon, 31 Aug 2015 13:17:08 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> > Now that interfaces got created, we need to fix the entity
> > namespace.
> >
> > So, let's create a consistent new namespace and add backward
> > compatibility macros to keep the old namespace preserved.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> >
> > diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> > index 14d32cdcdd47..88013d1a2c39 100644
> > --- a/drivers/media/dvb-core/dvbdev.c
> > +++ b/drivers/media/dvb-core/dvbdev.c
> > @@ -230,17 +230,17 @@ static void dvb_create_media_entity(struct dvb_device *dvbdev,
> >
> > switch (type) {
> > case DVB_DEVICE_FRONTEND:
> > - dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
> > + dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMOD;
> > dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> > dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> > break;
> > case DVB_DEVICE_DEMUX:
> > - dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
> > + dvbdev->entity->type = MEDIA_ENT_T_DVB_DEMUX;
> > dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> > dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> > break;
> > case DVB_DEVICE_CA:
> > - dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
> > + dvbdev->entity->type = MEDIA_ENT_T_DVB_CA;
> > dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> > dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> > break;
> > @@ -439,7 +439,7 @@ EXPORT_SYMBOL(dvb_unregister_device);
> > void dvb_create_media_graph(struct dvb_adapter *adap)
> > {
> > struct media_device *mdev = adap->mdev;
> > - struct media_entity *entity, *tuner = NULL, *fe = NULL;
> > + struct media_entity *entity, *tuner = NULL, *demod = NULL;
> > struct media_entity *demux = NULL, *dvr = NULL, *ca = NULL;
> > struct media_interface *intf;
> >
> > @@ -451,26 +451,26 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
> > case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
> > tuner = entity;
> > break;
> > - case MEDIA_ENT_T_DEVNODE_DVB_FE:
> > - fe = entity;
> > + case MEDIA_ENT_T_DVB_DEMOD:
> > + demod = entity;
> > break;
> > - case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
> > + case MEDIA_ENT_T_DVB_DEMUX:
> > demux = entity;
> > break;
> > - case MEDIA_ENT_T_DEVNODE_DVB_DVR:
> > + case MEDIA_ENT_T_DVB_TSOUT:
> > dvr = entity;
> > break;
> > - case MEDIA_ENT_T_DEVNODE_DVB_CA:
> > + case MEDIA_ENT_T_DVB_CA:
> > ca = entity;
> > break;
> > }
> > }
> >
> > - if (tuner && fe)
> > - media_create_pad_link(tuner, 0, fe, 0, 0);
> > + if (tuner && demod)
> > + media_create_pad_link(tuner, 0, demod, 0, 0);
> >
> > - if (fe && demux)
> > - media_create_pad_link(fe, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
> > + if (demod && demux)
> > + media_create_pad_link(demod, 1, demux, 0, MEDIA_LNK_FL_ENABLED);
> >
> > if (demux && dvr)
> > media_create_pad_link(demux, 1, dvr, 0, MEDIA_LNK_FL_ENABLED);
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index aca828709bad..3bbda409353f 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -42,31 +42,71 @@ struct media_device_info {
> >
> > #define MEDIA_ENT_ID_FLAG_NEXT (1 << 31)
> >
> > +/*
> > + * Base numbers for entity types
> > + *
> > + * Please notice that the huge gap of 16 bits for each base is overkill!
> > + * 8 bits is more than enough to avoid starving entity types for each
> > + * subsystem.
> > + *
> > + * However, It is kept this way just to avoid binary breakages with the
> > + * namespace provided on legacy versions of this header.
> > + */
> > +#define MEDIA_ENT_T_DVB_BASE 0x00000001
>
> I would change this to 0x00000000, see follow-up comment later for why.
>
> > +#define MEDIA_ENT_T_V4L2_BASE 0x00010000
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_BASE 0x00020000
> > +
> > +/*
> > + * V4L2 entities - Those are used for DMA (mmap/DMABUF) and
> > + * read()/write() data I/O associated with the V4L2 devnodes.
> > + */
> > +#define MEDIA_ENT_T_V4L2_VIDEO (MEDIA_ENT_T_V4L2_BASE + 1)
> > + /*
> > + * Please notice that numbers between MEDIA_ENT_T_V4L2_BASE + 2 and
> > + * MEDIA_ENT_T_V4L2_BASE + 4 can't be used, as those values used
> > + * to be declared for FB, ALSA and DVB entities.
> > + * As those values were never actually used in practice, we're just
> > + * adding them as backward compatibility macros and keeping the
> > + * numberspace clean here. This way, we avoid breaking compilation,
> > + * in the case of having some userspace application using the old
> > + * symbols.
> > + */
> > +#define MEDIA_ENT_T_V4L2_VBI (MEDIA_ENT_T_V4L2_BASE + 5)
> > + /* for TX radio, as RX is done via either ALSA or wire */
> > +#define MEDIA_ENT_T_V4L2_RADIO (MEDIA_ENT_T_V4L2_BASE + 6)
>
> But TX is also done via either ALSA or wire. This shouldn't be needed.
OK. I'll drop it.
>
> > +#define MEDIA_ENT_T_V4L2_SWRADIO (MEDIA_ENT_T_V4L2_BASE + 7)
>
> How about MEDIA_ENT_T_DVB_IO_* and MEDIA_ENT_T_V4L2_IO_* to indicate that
> this entity deals with data I/O?
>
> Or, perhaps even better, MEDIA_ENT_T_IO_DVB_ and MEDIA_ENT_T_IO_V4L2_.
Works for me.
>
> Entities should do something, and just saying 'V4L2_VIDEO' doesn't really convey
> that meaning. It is also very easy to confuse with INTF_T_V4L_* types. BTW, we
> should decide whether V4L2 or V4L is used here (interfaces now use V4L, entities
> V4L2). Since entities already use V4L2, I think the interface defines should
> use V4L2 as well.
Yes, agreed. We actually need to discuss a little more about
namespacing and do some additional renaming stuff.
For example, calling a tuner entity as MEDIA_ENT_T_V4L2_SUBDEV_TUNER
is wrong, because the tuner can be used only at the DVB side.
So, both V4L2 and SUBDEV prefixes there are wrong. Yet, this should be
under the V4L2_SUBDEV range to avoid breaking userspace.
>
> > +
> > +/* V4L2 Sub-device entities */
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 1)
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 2)
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_LENS (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 3)
> > + /* A converter of analogue video to its digital representation. */
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 4)
> > + /* Tuner entity is actually both V4L2 and DVB subdev */
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 5)
> > +
> > +/* DVB entities */
> > +#define MEDIA_ENT_T_DVB_DEMOD (MEDIA_ENT_T_DVB_BASE)
>
> After changing DVB_BASE to 0, change this to DVB_BASE + 1, and adjust the other DVB
> entity types accordingly.
>
> This keeps the base defines consistent (i.e. the lower 16 bits are always 0).
>
> It surprised me when reading this patch, so I'm probably not the only one.
This is another thing for discussion: keeping the MEDIA_ENT_T_foo_BASE
unused opens space for API abuse.
There are several entities at OMAP3 driver, for example, that keeps
entity->type undefined. So, they got whatever is the default at
v4l2-device.c (before this series: MEDIA_ENT_T_V4L2_SUBDEV, after
that: MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN):
$ media-ctl --print-t|grep -B1 Unknown
- entity 1: OMAP3 ISP CCP2 (2 pads, 2 links)
type V4L2 subdev subtype Unknown flags 0
--
- entity 3: OMAP3 ISP CSI2a (2 pads, 2 links)
type V4L2 subdev subtype Unknown flags 0
--
- entity 5: OMAP3 ISP CCDC (3 pads, 8 links)
type V4L2 subdev subtype Unknown flags 0
--
- entity 7: OMAP3 ISP preview (2 pads, 4 links)
type V4L2 subdev subtype Unknown flags 0
--
- entity 10: OMAP3 ISP resizer (2 pads, 4 links)
type V4L2 subdev subtype Unknown flags 0
--
- entity 13: OMAP3 ISP AEWB (1 pad, 1 link)
type V4L2 subdev subtype Unknown flags 0
--
- entity 14: OMAP3 ISP AF (1 pad, 1 link)
type V4L2 subdev subtype Unknown flags 0
--
- entity 15: OMAP3 ISP histogram (1 pad, 1 link)
type V4L2 subdev subtype Unknown flags 0
I guess all the above entities are processing units, so they
should have, instead, some type like:
MEDIA_ENT_T_V4L2_SUBDEV_PROCESSING
Or, even some of the above would actually deserves to have an specific
type, like:
MEDIA_ENT_T_V4L2_SUBDEV_HISTOGRAM
MEDIA_ENT_T_V4L2_SUBDEV_RESIZER
...
Let's try to find some time to discuss the entities namespace on IRC during
this week.
Regards,
Mauro
^ permalink raw reply
* Re: [PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl
From: Mauro Carvalho Chehab @ 2015-08-31 13:35 UTC (permalink / raw)
To: Hans Verkuil
Cc: Linux Media Mailing List, Mauro Carvalho Chehab,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <55E441EA.4020206-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
Em Mon, 31 Aug 2015 14:00:42 +0200
Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> escreveu:
> On 08/30/2015 05:06 AM, Mauro Carvalho Chehab wrote:
> > Add a new ioctl that will report the entire topology on
> > one go.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> >
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index 756e1960fd7f..358a0c6b1f86 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -181,6 +181,8 @@ struct media_interface {
> > */
> > struct media_intf_devnode {
> > struct media_interface intf;
> > +
> > + /* Should match the fields at media_v2_intf_devnode */
> > u32 major;
> > u32 minor;
> > };
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index 4186891e5e81..fa0b68e670b0 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -251,11 +251,94 @@ struct media_links_enum {
> > #define MEDIA_INTF_T_ALSA_RAWMIDI (MEDIA_INTF_T_ALSA_BASE + 4)
> > #define MEDIA_INTF_T_ALSA_HWDEP (MEDIA_INTF_T_ALSA_BASE + 5)
> >
> > -/* TBD: declare the structs needed for the new G_TOPOLOGY ioctl */
> > +/*
> > + * MC next gen API definitions
> > + *
> > + * NOTE: The declarations below are close to the MC RFC for the Media
> > + * Controller, the next generation. Yet, there are a few adjustments
> > + * to do, as we want to be able to have a functional API before
> > + * the MC properties change. Those will be properly marked below.
> > + * Please also notice that I removed "num_pads", "num_links",
> > + * from the proposal, as a proper userspace application will likely
> > + * use lists for pads/links, just as we intend todo in Kernelspace.
>
> s/todo/to do/
>
> > + * The API definition should be freed from fields that are bound to
> > + * some specific data structure.
> > + *
> > + * FIXME: Currently, I opted to name the new types as "media_v2", as this
> > + * won't cause any conflict with the Kernelspace namespace, nor with
> > + * the previous kAPI media_*_desc namespace. This can be changed
> > + * latter, before the adding this API upstream.
>
> s/latter/later/ :-)
>
> I think this comment belongs to the commit log and not in this header.
True, but I opted to keep it here for now to produce some discussions ;)
I'm actually in doubt if we should rename the flags as proposed below,
and use the newer flags only at G_TOPOLOGY or if we should keep the same
namespace for them and accept newer flags with legacy ioctls.
>
> > + */
> > +
> > +
> > +#define MEDIA_NEW_LNK_FL_ENABLED MEDIA_LNK_FL_ENABLED
> > +#define MEDIA_NEW_LNK_FL_IMMUTABLE MEDIA_LNK_FL_IMMUTABLE
> > +#define MEDIA_NEW_LNK_FL_DYNAMIC MEDIA_NEW_FL_DYNAMIC
> > +#define MEDIA_NEW_LNK_FL_INTERFACE_LINK (1 << 3)
>
> Shouldn't this be MEDIA_V2_ instead of MEDIA_NEW_?
>
> Do we need the INTERFACE_LINK flag? You can deduce it by checking the
> ID type.
Yes, this can be deduced from the type of the objects inside the link.
I guess I added it because of some comment from your media.h RFC
proposal.
Right now, I'm using it at the application to better represent the graph
elements:
http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-utils.git/tree/contrib/test/mc_nextgen_test.c?h=mc-next-gen&id=fdc16ece9732c94cfa76eee86978158c5976c00a#n438
http://git.linuxtv.org/cgit.cgi/mchehab/experimental-v4l-utils.git/tree/contrib/test/mc_nextgen_test.c?h=mc-next-gen&id=fdc16ece9732c94cfa76eee86978158c5976c00a#n444
But it could, instead, be doing something like:
if media_type(link->gobj1.id == MEDIA_GRAPH_PAD)
link_is_pad = true;
else
link_is_pad = false;
Btw, I'm using the same type for both data and interface links, as
I don't see any reason why to differentiate internally: they all share
the same linked list at mdev and the same object ID range.
>
> I don't have a clear preference one way or another, just wondering about the
> reason for adding it.
>
> > +
> > +struct media_v2_entity {
> > + __u32 id;
> > + char name[64]; /* FIXME: move to a property? (RFC says so) */
> > + __u16 reserved[14];
> > +};
> > +
> > +/* Should match the specific fields at media_intf_devnode */
> > +struct media_v2_intf_devnode {
> > + __u32 major;
> > + __u32 minor;
> > +};
> > +
> > +struct media_v2_interface {
> > + __u32 id;
> > + __u32 intf_type;
> > + __u32 flags;
> > + __u32 reserved[9];
> > +
> > + union {
> > + struct media_v2_intf_devnode devnode;
> > + __u32 raw[16];
> > + };
> > +};
> > +
> > +struct media_v2_pad {
> > + __u32 id;
> > + __u32 entity_id;
> > + __u32 flags;
> > + __u16 reserved[9];
> > +};
> > +
> > +struct media_v2_link {
> > + __u32 id;
> > + __u32 source_id;
> > + __u32 sink_id;
>
> Like in media_link I would use a union here as well to be able to refer to
> source/sink_id and entity/interface_id.
That would be overkill, and won't help.
Unions make the code harder to read and kernel-doc-nano doesn't like unions
very much.
Ok, there are some cases where it helps, but there's no good reason
for doing it here.
If you don't like the name, let's just rename it to something else.
>
> > + __u32 flags;
> > + __u32 reserved[5];
> > +};
> > +
> > +struct media_v2_topology {
> > + __u32 topology_version;
> > +
> > + __u32 num_entities;
> > + struct media_v2_entity *entities;
> > +
> > + __u32 num_interfaces;
> > + struct media_v2_interface *interfaces;
> > +
> > + __u32 num_pads;
> > + struct media_v2_pad *pads;
> > +
> > + __u32 num_links;
> > + struct media_v2_link *links;
> > +
> > + __u32 reserved[64];
>
> As mentioned before: use this instead to prevent horrible 32/64 bit arch
> compat code:
>
> struct {
> __u32 reserved_num;
> void *reserved_ptr;
> } reserved_types[16];
> __u32 reserved[8];
>
> Sizes for these arrays are TBD.
OK. Sorry, I forgot to do this change.
>
> > +};
> > +
> > +/* ioctls */
> >
> > #define MEDIA_IOC_DEVICE_INFO _IOWR('|', 0x00, struct media_device_info)
> > #define MEDIA_IOC_ENUM_ENTITIES _IOWR('|', 0x01, struct media_entity_desc)
> > #define MEDIA_IOC_ENUM_LINKS _IOWR('|', 0x02, struct media_links_enum)
> > #define MEDIA_IOC_SETUP_LINK _IOWR('|', 0x03, struct media_link_desc)
> > +#define MEDIA_IOC_G_TOPOLOGY _IOWR('|', 0x04, struct media_v2_topology)
> >
> > #endif /* __LINUX_MEDIA_H */
> >
>
> Regards,
>
> Hans
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC v6 10/40] richacl: Permission check algorithm
From: J. Bruce Fields @ 2015-08-31 15:23 UTC (permalink / raw)
To: Andreas Grünbacher
Cc: Linux Kernel Mailing List, Linux FS-devel Mailing List,
Linux NFS Mailing List, Linux API Mailing List,
linux-cifs-u79uwXL29TY76Z2rM5mHXA, LSM List, Andreas Gruenbacher
In-Reply-To: <CAHpGcMJT0kQyrvCkur0csCmRn-LwUOJcRWqAfLaqCpNd2b8UXA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Sat, Aug 29, 2015 at 12:06:22AM +0200, Andreas Grünbacher wrote:
> 2015-08-28 23:49 GMT+02:00 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> > On Tue, Aug 04, 2015 at 01:53:08PM +0200, Andreas Gruenbacher wrote:
> >> + /*
> >> + * We don't care which class the process is in when the acl is
> >> + * not masked.
> >> + */
> >> + in_owner_or_group_class = 1;
> >
> > So why bother setting it at all, then? Oh, I see, it lets you break out
> > of the loop below earlier. OK.
>
> Comment changed to:
>
> /*
> * When the acl is not masked, there is no need to determine if
> * the process is in the group class and we can earlier break
> * out of the loop below.
> */
makes sense, just replace "earlier break out" by "break out earlier".
>
> > Patch makes sense to me, ACK.
>
> Thanks,
> Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH 1/9] powerpc: allocate sys_membarrier system call number
From: Mathieu Desnoyers @ 2015-08-31 15:32 UTC (permalink / raw)
To: Michael Ellerman
Cc: Andrew Morton, linux-api, linux-kernel, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev
In-Reply-To: <1441004045.5735.8.camel@ellerman.id.au>
----- On Aug 31, 2015, at 2:54 AM, Michael Ellerman mpe@ellerman.id.au wrote:
> On Thu, 2015-08-27 at 13:56 -0400, Mathieu Desnoyers wrote:
>> Allow it to be used from SPU, since it should not have unwanted
>> side-effects.
>>
>> [ Untested on this architecture. To try it out: fetch linux-next/akpm,
>> apply this patch, build/run a membarrier-enabled kernel, and do make
>> kselftest. ]
>>
>> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> CC: Andrew Morton <akpm@linux-foundation.org>
>> CC: linux-api@vger.kernel.org
>> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> CC: Paul Mackerras <paulus@samba.org>
>> CC: Michael Ellerman <mpe@ellerman.id.au>
>> CC: linuxppc-dev@lists.ozlabs.org
>
> Thanks.
>
> I get:
>
> $ ./membarrier_test
> membarrier MEMBARRIER_CMD_QUERY syscall available.
> membarrier: MEMBARRIER_CMD_SHARED success.
> membarrier: tests done!
>
> Which looks good.
>
> Assuming the membarrier support hits 4.3, I'll take this via my tree for 4.3
> also.
Great! I'll take care of your comments about self-tests,
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Confidential Business Negotiation!!
From: Mr. Lassine Diawara @ 2015-08-31 21:54 UTC (permalink / raw)
--
Greetings from Dr. Lassine Diawara.
Assalamu Alaikum,
Permit me to inform you of my desire of going into
business relationship with you, I know this message will sounds very
strange to you, but I have no choice than to risk my life to choose a
stranger like you I have never seen before. We leave in a global
world today, and with the help of internet technology one can easily
communicate with one another all around the world without knowing him
or she in person, what matters will be the trust and the
Information’s you got about the person. In this case we can understand
our self and work like one family.
My Name is Dr. Lassine Diawara, the senior Auditor In charge of
Foreign Remittance Unit, Bank of Africa (BOA). I'm from Ouagadougou,
Burkina Faso, West Africa. My reason for contacting you is to transfer
an abandoned $15.5M to your account.
The owner of this fund died since 2003 with his Next Of Kin. I want to
present you to the bank as the Next of Kin/beneficiary of this fund.
Further details of the transaction shall be forwarded to you as soon
as I receive your return mail indicating your interest.
I will like you to provide immediately the below information's, to
enable me use it and get you next of kin application form from my
bank.
1. Full Name:......................
2.Full Address:...................
3.Telephone Number:...............
4.Country:........................
5.Occupation:.....................
6.Age:............................
7.Sex:............................
As soon as you reply, I will let you know the next steps and
proceduresand more details to follow in order to finalize this
transaction immediately.
Yours Sincerely,
Dr Lassine
^ permalink raw reply
* Re: [RFC PATCH 1/9] powerpc: allocate sys_membarrier system call number
From: Michael Ellerman @ 2015-09-01 1:00 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Andrew Morton, linux-api, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
Benjamin Herrenschmidt, Paul Mackerras,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <131898944.32688.1441035174419.JavaMail.zimbra-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
On Mon, 2015-08-31 at 15:32 +0000, Mathieu Desnoyers wrote:
> ----- On Aug 31, 2015, at 2:54 AM, Michael Ellerman mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org wrote:
>
> > On Thu, 2015-08-27 at 13:56 -0400, Mathieu Desnoyers wrote:
> >> Allow it to be used from SPU, since it should not have unwanted
> >> side-effects.
> > $ ./membarrier_test
> > membarrier MEMBARRIER_CMD_QUERY syscall available.
> > membarrier: MEMBARRIER_CMD_SHARED success.
> > membarrier: tests done!
> >
> > Which looks good.
> >
> > Assuming the membarrier support hits 4.3, I'll take this via my tree for 4.3
> > also.
>
> Great! I'll take care of your comments about self-tests,
Thanks. None of them are critical so we can always fix them later.
cheers
^ permalink raw reply
* Re: [PATCH v8 3/6] mm: Introduce VM_LOCKONFAULT
From: Eric B Munson @ 2015-09-01 12:51 UTC (permalink / raw)
To: Michal Hocko, Eric B Munson
Cc: Andrew Morton, Vlastimil Babka, Jonathan Corbet,
Kirill A. Shutemov, linux-kernel, linux-mm, linux-api
In-Reply-To: <20150828141829.GD5301@dhcp22.suse.cz>
On August 28, 2015 10:18:30 AM EDT, Michal Hocko <mhocko@kernel.org> wrote:
>
>Why do we need to export this? Neither of the consumers care and should
>care. VM_LOCKONFAULT should never be set without VM_LOCKED which is the
>only thing that we should care about.
I am out of the office and I don't know if I will be back in time to respin this series without this hunk. Andrew, can you remove the export of VM_LOCKONFAULT to the rmap code?
Thank,
Eric
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v2 0/3] SysFS driver for QEMU fw_cfg device
From: Gabriel L. Somlo @ 2015-09-01 16:11 UTC (permalink / raw)
To: Christopher Covington
Cc: Ard Biesheuvel,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Richard W.M. Jones, Jordan Justen,
x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, QEMU Developers,
gleb-RmZWMc9puTNJc61us3aD9laTQe2KTcn/, Matt Fleming,
kernelnewbies-7JyXY6prKcjpASu1u0TL5ti2O/JbrIOy, Gerd Hoffmann,
Paolo Bonzini, Laszlo Ersek,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
ralf-6z/3iImG2C8G8FEW9MqTrA, zajec5-Re5JQEeQqe8AvxtiuMwx3w,
paul-DWxLp4Yu+b8AvxtiuMwx3w, galak-sgV2jX0FEOL9JmXXK+q4OQ,
linux-api-u79uwXL29TY76Z2rM5mHXA, Leif Lindholm
In-Reply-To: <55DE0227.8090905-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Hi Christopher,
On Wed, Aug 26, 2015 at 02:15:03PM -0400, Christopher Covington wrote:
> On 08/19/2015 04:49 PM, Gabriel L. Somlo wrote:
> > On Wed, Aug 19, 2015 at 11:42:02AM +0200, Ard Biesheuvel wrote:
> >> On 19 August 2015 at 11:38, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >>> From: "Gabriel L. Somlo" <somlo-D+Gtc/HYRWM@public.gmane.org>
> >>>> Several different architectures supported by QEMU are set up with a
> >>>> "firmware configuration" (fw_cfg) device, used to pass configuration
> >>>> "blobs" into the guest by the host running QEMU.
> >>>>
> >>>> Historically, these config blobs were mostly of interest to the guest
> >>>> BIOS, but since QEMU v2.4 it is possible to insert arbitrary blobs via
> >>>> the command line, which makes them potentially interesting to userspace
> >>>> (e.g. for passing early boot environment variables, etc.).
> >>>>
> >>>
> >>> Does 'potentially interesting' mean you have a use case? Could you elaborate?
> >
> > My personal one would be something like:
> >
> > cat > guestinfo.txt << EOT
> > KEY1="val1"
> > KEY2="val2"
> > ...
> > EOT
> >
> > qemu-system-x86_64 ... -fw-cfg name="opt/guestinfo",file=./guestinfo.txt ...
> >
> > Then, from inside the guest:
> >
> > . /sys/firmware/qemu_fw_cfg/by_name/opt/guestinfo/raw
> >
> > do_something_with $KEY1 $KEY2
> > ...
> >
> > But I'm thinking this is only one of the many positive things one
> > could do with the ability to access random host-supplied blobs from
> > guest userspace :)
>
> I do this with kernel parameters:
>
> host:
> qemu-system-aarch64 -append="KEY1=val1 KEY2=val2"
>
> guest:
> KEY1=`sed -nr s/.*KEY1=([^ ]+).*/\1/ /proc/cmdline`
> KEY2=`sed -nr s/.*KEY2=([^ ]+).*/\1/ /proc/cmdline`
>
> do_something_with $KEY1 $KEY2
>
> In practice it's just script=hostfile, where hostfile is available to the
> guest via a 9P passthrough filesystem mount.
>
> While quite architecture specific, I've also previously used an
> "angel-cmdline" tool for similar purposes. Peter's recent semihosting patches
> support such a tool for AArch64. (On AArch32 upstream QEMU disallows
> semihosting from userspace.)
>
> Before I had 9P on all the simulators I regularly ran, I used a semihosting
> based "angel-load" tool.
Someone (maybe it was you) did suggest this during an early thread on
the QEMU dev list. I had considered this, then thought about
piggybacking on smbios (e.g. the type 40 "additional information"
table), but then realized "wait, smbios is currently being inserted
into the guest via fw_cfg, so maybe direct fw_cfg blob transfer *is*
the most asynchronous and out-of-band way I can do this... :)
True, writing a fw_cfg driver is still Linux-specific (leaves out
Windows, which is something I still care about), but dumping a fw_cfg
blob using 'cat' or 'cp' on linux has a certain appeal :)
Yeah, the immediate use case I have personally can be worked around
with kernel command line args, in a slightly less out-of-band, but
still quite serviceable way. I'm thinking a fw_cfg driver is just a
flexible way to make other use cases possible in a (IMHO) neat and
clean way...
Plus, it got me to learn about kobjects, now studying DTs on ARM, so
it's fun ;)
Thanks,
--Gabriel
^ permalink raw reply
* Re: [PATCH 2/3] selftests: add membarrier syscall test
From: Mathieu Desnoyers @ 2015-09-01 17:11 UTC (permalink / raw)
To: Michael Ellerman
Cc: Andrew Morton, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-api,
Pranith Kumar
In-Reply-To: <1441004040.5735.7.camel-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
----- On Aug 31, 2015, at 2:54 AM, Michael Ellerman mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org wrote:
> On Fri, 2015-07-10 at 16:58 -0400, Mathieu Desnoyers wrote:
>> From: Pranith Kumar <bobby.prani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> This patch adds a self test for the membarrier system call.
>>
>> CC: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
>
> Sorry I only just saw this due to some over zealous filtering on my end.
>
>
>> diff --git a/tools/testing/selftests/membarrier/Makefile
>> b/tools/testing/selftests/membarrier/Makefile
>> new file mode 100644
>> index 0000000..877a503
>> --- /dev/null
>> +++ b/tools/testing/selftests/membarrier/Makefile
>> @@ -0,0 +1,11 @@
>> +CFLAGS += -g -I../../../../usr/include/
>> +
>> +all:
>> + $(CC) $(CFLAGS) membarrier_test.c -o membarrier_test
>>
>> +TEST_PROGS := membarrier_test
>
> You don't need to specify the rule, the implict one will do exactly the same,
> so you can just do:
>
> TEST_PROGS := membarrier_test
>
> all: $(TEST_PROGS)
>
>> diff --git a/tools/testing/selftests/membarrier/membarrier_test.c
>> b/tools/testing/selftests/membarrier/membarrier_test.c
>> new file mode 100644
>> index 0000000..3c9f217
>> --- /dev/null
>> +++ b/tools/testing/selftests/membarrier/membarrier_test.c
>> @@ -0,0 +1,71 @@
>> +#define _GNU_SOURCE
>> +#define __EXPORTED_HEADERS__
>
> Why are you exporting that?
>
> I suspect to try and get around the "Attempt to use kernel headers from user
> space" warning.
>
> But you're correctly building against the installed headers, not the kernel
> headers, so you don't need to do that.
Just to make sure I understand: should we expect that
everyone will issue "make headers_install" on their system
before doing a make kselftest ?
I see that a few selftests (e.g. memfd) are adding the
source tree include paths to the compiler include paths,
which I guess is to ensure that the kselftest will
work even if the system headers are not up to date.
Thanks,
Mathieu
>
>> +
>> +#include <linux/membarrier.h>
>> +#include <asm-generic/unistd.h>
>
> This should just be <unistd.h>
>
>
> cheers
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH 2/3] selftests: add membarrier syscall test
From: Andy Lutomirski @ 2015-09-01 18:32 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Michael Ellerman, Andrew Morton,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-api,
Pranith Kumar
In-Reply-To: <1071313434.33305.1441127478843.JavaMail.zimbra-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
On Tue, Sep 1, 2015 at 10:11 AM, Mathieu Desnoyers
<mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> ----- On Aug 31, 2015, at 2:54 AM, Michael Ellerman mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org wrote:
>
>> On Fri, 2015-07-10 at 16:58 -0400, Mathieu Desnoyers wrote:
>>> From: Pranith Kumar <bobby.prani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>
>>> This patch adds a self test for the membarrier system call.
>>>
>>> CC: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
>>
>> Sorry I only just saw this due to some over zealous filtering on my end.
>>
>>
>>> diff --git a/tools/testing/selftests/membarrier/Makefile
>>> b/tools/testing/selftests/membarrier/Makefile
>>> new file mode 100644
>>> index 0000000..877a503
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/membarrier/Makefile
>>> @@ -0,0 +1,11 @@
>>> +CFLAGS += -g -I../../../../usr/include/
>>> +
>>> +all:
>>> + $(CC) $(CFLAGS) membarrier_test.c -o membarrier_test
>>>
>>> +TEST_PROGS := membarrier_test
>>
>> You don't need to specify the rule, the implict one will do exactly the same,
>> so you can just do:
>>
>> TEST_PROGS := membarrier_test
>>
>> all: $(TEST_PROGS)
>>
>>> diff --git a/tools/testing/selftests/membarrier/membarrier_test.c
>>> b/tools/testing/selftests/membarrier/membarrier_test.c
>>> new file mode 100644
>>> index 0000000..3c9f217
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/membarrier/membarrier_test.c
>>> @@ -0,0 +1,71 @@
>>> +#define _GNU_SOURCE
>>> +#define __EXPORTED_HEADERS__
>>
>> Why are you exporting that?
>>
>> I suspect to try and get around the "Attempt to use kernel headers from user
>> space" warning.
>>
>> But you're correctly building against the installed headers, not the kernel
>> headers, so you don't need to do that.
>
> Just to make sure I understand: should we expect that
> everyone will issue "make headers_install" on their system
> before doing a make kselftest ?
>
> I see that a few selftests (e.g. memfd) are adding the
> source tree include paths to the compiler include paths,
> which I guess is to ensure that the kselftest will
> work even if the system headers are not up to date.
It would be really nice if there were a clean way for selftests to
include the kernel headers. Perhaps make should build the exportable
headers somewhere as a dependency of kselftests.
--Andy
^ permalink raw reply
* Re: [RFC v6 09/40] richacl: Update the file masks in chmod()
From: J. Bruce Fields @ 2015-09-01 21:38 UTC (permalink / raw)
To: Andreas Gruenbacher
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-nfs-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-cifs-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA, Andreas Gruenbacher
In-Reply-To: <1438689218-6921-10-git-send-email-agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Aug 04, 2015 at 01:53:07PM +0200, Andreas Gruenbacher wrote:
> Doing a chmod() sets the file mode, which includes the file permission
> bits. When a file has a richacl, the permissions that the richacl
> grants need to be limited to what the new file permission bits allow.
>
> This is done by setting the file masks in the richacl to what the file
> permission bits map to. The richacl access check algorithm takes the
> file masks into account, which ensures that the richacl cannot grant too
> many permissions.
>
> It is possible to explicitly add permissions to the file masks which go
> beyond what the file permission bits can grant (like the
> RICHACE_WRITE_ACL permission). The POSIX.1 standard calls this an
> alternate file access control mechanism. A subsequent chmod() would
> ensure that those permissions are disabled again.
>
> Signed-off-by: Andreas Gruenbacher <agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> fs/richacl_base.c | 40 ++++++++++++++++++++++++++++++++++++++++
> include/linux/richacl.h | 1 +
> 2 files changed, 41 insertions(+)
>
> diff --git a/fs/richacl_base.c b/fs/richacl_base.c
> index 8426600..fd81158 100644
> --- a/fs/richacl_base.c
> +++ b/fs/richacl_base.c
> @@ -338,3 +338,43 @@ restart:
> acl->a_flags &= ~(RICHACL_WRITE_THROUGH | RICHACL_MASKED);
> }
> EXPORT_SYMBOL_GPL(richacl_compute_max_masks);
> +
> +/**
> + * richacl_chmod - update the file masks to reflect the new mode
> + * @mode: new file permission bits including the file type
> + *
> + * Return a copy of @acl where the file masks have been replaced by the file
> + * masks corresponding to the file permission bits in @mode, or returns @acl
> + * itself if the file masks are already up to date. Takes over a reference
> + * to @acl.
> + */
> +struct richacl *
> +richacl_chmod(struct richacl *acl, mode_t mode)
> +{
> + unsigned int x = S_ISDIR(mode) ? 0 : RICHACE_DELETE_CHILD;
> + unsigned int owner_mask, group_mask, other_mask;
> + struct richacl *clone;
> +
> + owner_mask = richacl_mode_to_mask(mode >> 6) & ~x;
> + group_mask = richacl_mode_to_mask(mode >> 3) & ~x;
> + other_mask = richacl_mode_to_mask(mode) & ~x;
> +
> + if (acl->a_owner_mask == owner_mask &&
> + acl->a_group_mask == group_mask &&
> + acl->a_other_mask == other_mask &&
> + (acl->a_flags & RICHACL_MASKED))
Do you need to require WRITE_THROUGH here too?
--b.
> + return acl;
> +
> + clone = richacl_clone(acl, GFP_KERNEL);
> + richacl_put(acl);
> + if (!clone)
> + return ERR_PTR(-ENOMEM);
> +
> + clone->a_flags |= (RICHACL_WRITE_THROUGH | RICHACL_MASKED);
> + clone->a_owner_mask = owner_mask;
> + clone->a_group_mask = group_mask;
> + clone->a_other_mask = other_mask;
> +
> + return clone;
> +}
> +EXPORT_SYMBOL_GPL(richacl_chmod);
> diff --git a/include/linux/richacl.h b/include/linux/richacl.h
> index 3d719db..8df6d93 100644
> --- a/include/linux/richacl.h
> +++ b/include/linux/richacl.h
> @@ -304,5 +304,6 @@ extern int richacl_masks_to_mode(const struct richacl *);
> extern unsigned int richacl_mode_to_mask(mode_t);
> extern unsigned int richacl_want_to_mask(unsigned int);
> extern void richacl_compute_max_masks(struct richacl *, kuid_t);
> +extern struct richacl *richacl_chmod(struct richacl *, mode_t);
>
> #endif /* __RICHACL_H */
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices
From: Yinghai Lu @ 2015-09-01 23:14 UTC (permalink / raw)
To: Sean O. Stalley
Cc: Bjorn Helgaas, Rajat Jain, Michael S. Tsirkin,
Rafał Miłecki, gong.chen@linux.intel.com,
linux-pci@vger.kernel.org, Linux Kernel Mailing List, linux-api
In-Reply-To: <1440089947-2839-3-git-send-email-sean.stalley@intel.com>
On Thu, Aug 20, 2015 at 9:59 AM, Sean O. Stalley <sean.stalley@intel.com> wrote:
> Add support for devices using Enhanced Allocation entries instead of BARs.
> This patch allows the kernel to parse the EA Extended Capability structure
> in PCI configspace and claim the BAR-equivalent resources.
>
> Signed-off-by: Sean O. Stalley <sean.stalley@intel.com>
> ---
> drivers/pci/pci.c | 219 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/pci/pci.h | 1 +
> drivers/pci/probe.c | 3 +
> 3 files changed, 223 insertions(+)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 0008c95..c8217a8 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
...
> +
> +/* Read an Enhanced Allocation (EA) entry */
> +static int pci_ea_read(struct pci_dev *dev, int offset)
> +{
...
> + res->name = pci_name(dev);
> + res->start = start;
> + res->end = end;
> + res->flags = flags;
> +
> + pci_ea_claim_resource(dev, res);
> +
> +out:
> + return offset + ent_size;
> +}
> +
> +/* Enhanced Allocation Initalization */
> +void pci_ea_init(struct pci_dev *dev)
> +{
...
> +
> + for (i = 0; i < num_ent; ++i) {
> + /* parse each EA entry */
> + dev_dbg(&dev->dev, "%s: parsing entry %i...\n", __func__, i);
> + offset = pci_ea_read(dev, offset);
> + }
> +}
> +
> static void pci_add_saved_cap(struct pci_dev *pci_dev,
> struct pci_cap_saved_state *new_cap)
> {
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index cefd636..4cadf35 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1522,6 +1522,9 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
>
> static void pci_init_capabilities(struct pci_dev *dev)
> {
> + /* Enhanced Allocation */
> + pci_ea_init(dev);
> +
> /* MSI/MSI-X list */
> pci_msi_init_pci_dev(dev);
>
Should not call pci_ea_claim_resource() that early.
For x86 and other arches, we call
pcibios_resource_survey/pcibios_allocate_bus_resouce/pcibios_allocate_resources
quite late.
Yinghai
^ permalink raw reply
* [PATCH] Remove #ifdef CONFIG_64BIT from all asm-generic/fcntl.h
From: Palmer Dabbelt @ 2015-09-02 0:10 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-arch, linux-api, linux-kernel, Palmer Dabbelt
From: Palmer Dabbelt <palmer.dabbelt@eecs.berkeley.edu>
When working on the RISC-V port I noticed that F_SETLK64 was being
defined on our 64-bit platform, despite our port being so new that
we've only ever had the 64-bit file ops. Since there's not compat
layer for these, this causes fcntl to bail out.
It turns out that one of the ways in with F_SETLK64 was being defined
(there's some more in glibc, but that's a whole different story... :))
is the result of CONFIG_64BIT showing up in this user-visible header.
<asm-generic/bitsperlong.h> confirms this isn't sane, so I replaced it
with a __BITS_PER_LONG check.
I went ahead and grep'd for any more of these (with
headers_install_all), and this was the only one I found.
Signed-off-by: Palmer Dabbelt <palmer.dabbelt@eecs.berkeley.edu>
Reviewed-by: Andrew Waterman <waterman@eecs.berkeley.edu>
Reviewed-by: Albert Ou <aou@eecs.berkeley.edu>
---
include/uapi/asm-generic/fcntl.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index e063effe0cc1..14a5c8237d84 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -1,6 +1,7 @@
#ifndef _ASM_GENERIC_FCNTL_H
#define _ASM_GENERIC_FCNTL_H
+#include <asm/bitsperlong.h>
#include <linux/types.h>
/*
@@ -115,7 +116,7 @@
#define F_GETSIG 11 /* for sockets. */
#endif
-#ifndef CONFIG_64BIT
+#if (__BITS_PER_LONG == 32)
#ifndef F_GETLK64
#define F_GETLK64 12 /* using 'struct flock64' */
#define F_SETLK64 13
--
2.4.6
^ permalink raw reply related
* Re: [PATCH v4 3/4] of: overlay: add per overlay sysfs attributes
From: Rob Herring @ 2015-09-02 2:35 UTC (permalink / raw)
To: Pantelis Antoniou
Cc: Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
Guenter Roeck, Greg Kroah-Hartman,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Pantelis Antoniou
In-Reply-To: <1434137896-13806-4-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On Fri, Jun 12, 2015 at 2:38 PM, Pantelis Antoniou
<pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> The two default overlay attributes are:
>
> * A targets sysfs attribute listing the targets of the installed
> overlay. The targets list the path on the kernel's device tree
> where each overlay fragment is applied to
>
> * A per overlay can_remove sysfs attribute that reports whether
> the overlay can be removed or not due to another overlapping overlay.
What will a user do with this information? Can this just be debugfs?
Don't we already have the targets in the overlay itself? If we are
going to provide some overlay info, shouldn't we provide all of it
(i.e. the FDT). For example, what do we do on a kexec?
Rob
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> ---
> drivers/of/overlay.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 37ec858..747568f 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -373,8 +373,61 @@ static const struct attribute *overlay_global_attrs[] = {
> NULL
> };
>
> +static ssize_t can_remove_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct of_overlay *ov = kobj_to_overlay(kobj);
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", overlay_removal_is_ok(ov));
> +}
> +
> +static ssize_t targets_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct of_overlay *ov = kobj_to_overlay(kobj);
> + struct of_overlay_info *ovinfo;
> + char *s, *e;
> + ssize_t ret;
> + int i, len;
> +
> + s = buf;
> + e = buf + PAGE_SIZE;
> +
> + mutex_lock(&of_mutex);
> +
> + /* targets */
> + for (i = 0; i < ov->count; i++) {
> + ovinfo = &ov->ovinfo_tab[i];
> +
> + len = snprintf(s, e - s, "%s\n",
> + of_node_full_name(ovinfo->target));
> + if (len == 0) {
> + ret = -ENOSPC;
> + goto err;
> + }
> + s += len;
> + }
> +
> + /* the buffer is zero terminated */
> + ret = s - buf;
> +err:
> + mutex_unlock(&of_mutex);
> + return ret;
> +}
> +
> +static struct kobj_attribute can_remove_attr = __ATTR_RO(can_remove);
> +static struct kobj_attribute targets_attr = __ATTR_RO(targets);
> +
> +static struct attribute *overlay_attrs[] = {
> + &can_remove_attr.attr,
> + &targets_attr.attr,
> + NULL
> +};
> +
> static struct kobj_type of_overlay_ktype = {
> .release = of_overlay_release,
> + .sysfs_ops = &kobj_sysfs_ops, /* default kobj sysfs ops */
> + .default_attrs = overlay_attrs,
> };
>
> static struct kset *ov_kset;
> --
> 1.7.12
>
^ permalink raw reply
* Re: [PATCH v4 2/4] of: overlay: global sysfs enable attribute
From: Rob Herring @ 2015-09-02 2:43 UTC (permalink / raw)
To: Pantelis Antoniou, Grant Likely
Cc: Andrew Morton, Matt Porter, Koen Kooi, Guenter Roeck,
Greg Kroah-Hartman,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Pantelis Antoniou
In-Reply-To: <1434137896-13806-3-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
On Fri, Jun 12, 2015 at 2:38 PM, Pantelis Antoniou
<pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> A throw once master enable switch to protect against any
> further overlay applications if the administrator desires so.
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
I had some questioning whether we want more states than just on/off
like "only add sub-trees", but I've convinced myself it is better to
keep the kill switch simple. So I'm okay with patches 1 and 2 (and the
relevant part of 4 if you want to split it). I think #3 needs to be
part of how we expect userspace to interact with overlays.
Please respin and I will queue at least the kill switch for 4.4 unless
there are further comments.
Rob
> ---
> drivers/of/overlay.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index f17f5ef..37ec858 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -21,6 +21,7 @@
> #include <linux/err.h>
> #include <linux/idr.h>
> #include <linux/sysfs.h>
> +#include <linux/atomic.h>
>
> #include "of_private.h"
>
> @@ -55,8 +56,12 @@ struct of_overlay {
> struct kobject kobj;
> };
>
> +/* master enable switch; once set to 0 can't be re-enabled */
> +static atomic_t ov_enable = ATOMIC_INIT(1);
> +
> static int of_overlay_apply_one(struct of_overlay *ov,
> struct device_node *target, const struct device_node *overlay);
> +static int overlay_removal_is_ok(struct of_overlay *ov);
>
> static int of_overlay_apply_single_property(struct of_overlay *ov,
> struct device_node *target, struct property *prop)
> @@ -339,6 +344,35 @@ void of_overlay_release(struct kobject *kobj)
> kfree(ov);
> }
>
> +static ssize_t enable_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + return snprintf(buf, PAGE_SIZE, "%d\n", atomic_read(&ov_enable));
> +}
> +
> +static ssize_t enable_store(struct kobject *kobj,
> + struct kobj_attribute *attr, const char *buf, size_t count)
> +{
> + int ret;
> + bool new_enable;
> +
> + ret = strtobool(buf, &new_enable);
> + if (ret != 0)
> + return ret;
> + /* if we've disabled it, no going back */
> + if (atomic_read(&ov_enable) == 0)
> + return -EPERM;
> + atomic_set(&ov_enable, (int)new_enable);
> + return count;
> +}
> +
> +static struct kobj_attribute enable_attr = __ATTR_RW(enable);
> +
> +static const struct attribute *overlay_global_attrs[] = {
> + &enable_attr.attr,
> + NULL
> +};
> +
> static struct kobj_type of_overlay_ktype = {
> .release = of_overlay_release,
> };
> @@ -360,6 +394,10 @@ int of_overlay_create(struct device_node *tree)
> struct of_overlay *ov;
> int err, id;
>
> + /* administratively disabled */
> + if (!atomic_read(&ov_enable))
> + return -EPERM;
> +
> /* allocate the overlay structure */
> ov = kzalloc(sizeof(*ov), GFP_KERNEL);
> if (ov == NULL)
> @@ -596,5 +634,8 @@ int of_overlay_init(void)
> if (!ov_kset)
> return -ENOMEM;
>
> - return 0;
> + rc = sysfs_create_files(&ov_kset->kobj, overlay_global_attrs);
> + WARN(rc, "%s: error adding global attributes\n", __func__);
> +
> + return rc;
> }
> --
> 1.7.12
>
^ permalink raw reply
* Re: [RFC v6 09/40] richacl: Update the file masks in chmod()
From: Andreas Grünbacher @ 2015-09-02 6:08 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Linux Kernel Mailing List, Linux FS-devel Mailing List,
Linux NFS Mailing List, Linux API Mailing List,
linux-cifs-u79uwXL29TY76Z2rM5mHXA, LSM List, Andreas Gruenbacher
In-Reply-To: <20150901213816.GH10468-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2015-09-01 23:38 GMT+02:00 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> Do you need to require WRITE_THROUGH here too?
Yes, indeed.
Thanks,
Andreas
^ permalink raw reply
* Re: [RFC PATCH 3/9] arm64: allocate sys_membarrier system call number
From: Will Deacon @ 2015-09-02 10:10 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Andrew Morton, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Catalin Marinas
In-Reply-To: <1440698215-8355-4-git-send-email-mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
On Thu, Aug 27, 2015 at 06:56:49PM +0100, Mathieu Desnoyers wrote:
> arm64 sys_membarrier number is already wired for arm64 through
> asm-generic/unistd.h, but needs to be allocated separately for
> the 32-bit compability layer of arm64.
>
> [ Untested on this architecture. To try it out: fetch linux-next/akpm,
> apply this patch, build/run a membarrier-enabled kernel, and do make
> kselftest. ]
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
> CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> CC: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> CC: Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>
> CC: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>
> ---
> arch/arm64/include/asm/unistd32.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h
> index cef934a..d97be80 100644
> --- a/arch/arm64/include/asm/unistd32.h
> +++ b/arch/arm64/include/asm/unistd32.h
> @@ -797,3 +797,5 @@ __SYSCALL(__NR_memfd_create, sys_memfd_create)
> __SYSCALL(__NR_bpf, sys_bpf)
> #define __NR_execveat 387
> __SYSCALL(__NR_execveat, compat_sys_execveat)
> +#define __NR_membarrier 388
> +__SYSCALL(__NR_membarrier, sys_membarrier)
I think people have made similar comments for other architectures, but
please also updated __NR_compat_syscalls when adding new compat syscalls
here.
Will
^ permalink raw reply
* Re: [PATCH v6 3/6] task_isolation: support PR_TASK_ISOLATION_STRICT mode
From: Will Deacon @ 2015-09-02 10:13 UTC (permalink / raw)
To: Chris Metcalf
Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
Viresh Kumar, Catalin Marinas,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <55DDD6EA.3070307-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org>
On Wed, Aug 26, 2015 at 04:10:34PM +0100, Chris Metcalf wrote:
> On 08/26/2015 06:36 AM, Will Deacon wrote:
> > On Tue, Aug 25, 2015 at 08:55:52PM +0100, Chris Metcalf wrote:
> >> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> >> index d882b833dbdb..e3d83a12f3cf 100644
> >> --- a/arch/arm64/kernel/ptrace.c
> >> +++ b/arch/arm64/kernel/ptrace.c
> >> @@ -37,6 +37,7 @@
> >> #include <linux/regset.h>
> >> #include <linux/tracehook.h>
> >> #include <linux/elf.h>
> >> +#include <linux/isolation.h>
> >>
> >> #include <asm/compat.h>
> >> #include <asm/debug-monitors.h>
> >> @@ -1150,6 +1151,10 @@ static void tracehook_report_syscall(struct pt_regs *regs,
> >>
> >> asmlinkage int syscall_trace_enter(struct pt_regs *regs)
> >> {
> >> + /* Ensure we report task_isolation violations in all circumstances. */
> >> + if (test_thread_flag(TIF_NOHZ) && task_isolation_strict())
> > This is going to force us to check TIF_NOHZ on the syscall slowpath even
> > when CONFIG_TASK_ISOLATION=n.
>
> Yes, good catch. I was thinking the "&& false" would suppress the TIF
> test but I forgot that test_bit() takes a volatile argument, so it gets
> evaluated even though the result isn't actually used.
>
> But I don't want to just reorder the two tests, because when isolation
> is enabled, testing TIF_NOHZ first is better. I think probably the right
> solution is just to put an #ifdef CONFIG_TASK_ISOLATION around that
> test, even though that is a little crufty. The alternative is to provide
> a task_isolation_configured() macro that just returns true or false, and
> make it a three-part "&&" test with that new macro first, but
> that seems a little crufty as well. Do you have a preference?
Maybe use IS_ENABLED(CONFIG_TASK_ISOLATION) ?
> >> + task_isolation_syscall(regs->syscallno);
> >> +
> >> /* Do the secure computing check first; failures should be fast. */
> > Here we have the usual priority problems with all the subsystems that
> > hook into the syscall path. If a prctl is later rewritten to a different
> > syscall, do you care about catching it? Either way, the comment about
> > doing secure computing "first" needs fixing.
>
> I admit I am unclear on the utility of rewriting prctl. My instinct is that
> we are trying to catch userspace invocations of prctl and allow them,
> and fail most everything else, so doing it pre-rewrite seems OK.
>
> I'm not sure if it makes sense to catch it before or after the
> secure computing check, though. On reflection maybe doing it
> afterwards makes more sense - what do you think?
I don't have a strong preference (I really hate all these hooks we have
on the syscall entry/exit path), but we do need to make sure that the
behaviour is consistent across architectures.
Will
^ permalink raw reply
* Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices
From: Sean O. Stalley @ 2015-09-02 17:46 UTC (permalink / raw)
To: Yinghai Lu
Cc: Bjorn Helgaas, Rajat Jain, Michael S. Tsirkin,
Rafał Miłecki,
gong.chen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org,
linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAE9FiQUQacSJXPqv9GQJ4AF0TN7uxcBiAYHVbHwvFhEfWgWKHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Thanks for taking a look Yinghai.
On Tue, Sep 01, 2015 at 04:14:08PM -0700, Yinghai Lu wrote:
> On Thu, Aug 20, 2015 at 9:59 AM, Sean O. Stalley <sean.stalley-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> > Add support for devices using Enhanced Allocation entries instead of BARs.
> > This patch allows the kernel to parse the EA Extended Capability structure
> > in PCI configspace and claim the BAR-equivalent resources.
> >
> > Signed-off-by: Sean O. Stalley <sean.stalley-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> > drivers/pci/pci.c | 219 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > drivers/pci/pci.h | 1 +
> > drivers/pci/probe.c | 3 +
> > 3 files changed, 223 insertions(+)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 0008c95..c8217a8 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> ...
> > +
> > +/* Read an Enhanced Allocation (EA) entry */
> > +static int pci_ea_read(struct pci_dev *dev, int offset)
> > +{
> ...
> > + res->name = pci_name(dev);
> > + res->start = start;
> > + res->end = end;
> > + res->flags = flags;
> > +
> > + pci_ea_claim_resource(dev, res);
> > +
> > +out:
> > + return offset + ent_size;
> > +}
> > +
> > +/* Enhanced Allocation Initalization */
> > +void pci_ea_init(struct pci_dev *dev)
> > +{
> ...
> > +
> > + for (i = 0; i < num_ent; ++i) {
> > + /* parse each EA entry */
> > + dev_dbg(&dev->dev, "%s: parsing entry %i...\n", __func__, i);
> > + offset = pci_ea_read(dev, offset);
> > + }
> > +}
> > +
> > static void pci_add_saved_cap(struct pci_dev *pci_dev,
> > struct pci_cap_saved_state *new_cap)
> > {
>
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index cefd636..4cadf35 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1522,6 +1522,9 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
> >
> > static void pci_init_capabilities(struct pci_dev *dev)
> > {
> > + /* Enhanced Allocation */
> > + pci_ea_init(dev);
> > +
> > /* MSI/MSI-X list */
> > pci_msi_init_pci_dev(dev);
> >
>
> Should not call pci_ea_claim_resource() that early.
Out of curiosity, why shouldn't resources be claimed that early?
EA resources are fixed by hardware. They are always there & will never move.
>
> For x86 and other arches, we call
> pcibios_resource_survey/pcibios_allocate_bus_resouce/pcibios_allocate_resources
> quite late.
Would it be better to modify pci_claim_resource() to support EA instead of adding pci_ea_claim_resource()?
That way, EA entries would be claimed at the same time as traditional BARs.
-Sean
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox