Re: [PATCH v8] drivers/misc: sysgenid: add system generation id driver

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Mar 08, 2021 at 05:03:58PM +0100, Alexander Graf wrote:
> 
> 
> On 08.03.21 15:36, Greg KH wrote:
> > 
> > On Mon, Mar 08, 2021 at 04:18:03PM +0200, Adrian Catangiu wrote:
> > > +static struct miscdevice sysgenid_misc = {
> > > +     .minor = MISC_DYNAMIC_MINOR,
> > > +     .name = "sysgenid",
> > > +     .fops = &fops,
> > > +};
> > 
> > Much cleaner, but:
> > 
> > > +static int __init sysgenid_init(void)
> > > +{
> > > +     int ret;
> > > +
> > > +     sysgenid_data.map_buf = get_zeroed_page(GFP_KERNEL);
> > > +     if (!sysgenid_data.map_buf)
> > > +             return -ENOMEM;
> > > +
> > > +     atomic_set(&sysgenid_data.generation_counter, 0);
> > > +     atomic_set(&sysgenid_data.outdated_watchers, 0);
> > > +     init_waitqueue_head(&sysgenid_data.read_waitq);
> > > +     init_waitqueue_head(&sysgenid_data.outdated_waitq);
> > > +     spin_lock_init(&sysgenid_data.lock);
> > > +
> > > +     ret = misc_register(&sysgenid_misc);
> > > +     if (ret < 0) {
> > > +             pr_err("misc_register() failed for sysgenid\n");
> > > +             goto err;
> > > +     }
> > > +
> > > +     return 0;
> > > +
> > > +err:
> > > +     free_pages(sysgenid_data.map_buf, 0);
> > > +     sysgenid_data.map_buf = 0;
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static void __exit sysgenid_exit(void)
> > > +{
> > > +     misc_deregister(&sysgenid_misc);
> > > +     free_pages(sysgenid_data.map_buf, 0);
> > > +     sysgenid_data.map_buf = 0;
> > > +}
> > > +
> > > +module_init(sysgenid_init);
> > > +module_exit(sysgenid_exit);
> > 
> > So you do this for any bit of hardware that happens to be out there?
> > Will that really work?  You do not have any hwid to trigger off of to
> > know that this is a valid device you can handle?
> 
> The interface is already useful in a pure container context where the
> generation change request is triggered by software.
> 
> And yes, there are hardware triggers, but Michael was quite unhappy about
> potential races between VMGenID change and SysGenID change and thus wanted
> to ideally separate the interfaces. So we went ahead and isolated the
> SysGenID one, as it's already useful as is.
> 
> Hardware drivers to inject change events into SysGenID can then follow
> later, for all different hardware platforms. But SysGenID as in this patch
> is a completely hardware agnostic concept.

Ok, this is going to play havoc with fuzzers and other "automated
testers", should be fun to watch!  :)

Let's queue this up and see what happens...

thanks,

greg k-h