Re: [PATCH 2/3] firmware: add offset to request_firmware_into_buf
On Thu, Aug 01, 2019 at 11:15:19AM -0700, Scott Branden wrote: > Hi Luis, > > On 2019-08-01 10:42 a.m., Luis Chamberlain wrote: > > On Thu, Aug 01, 2019 at 08:18:01AM +0200, Greg Kroah-Hartman wrote: > > > On Wed, Jul 31, 2019 at 05:18:32PM -0700, Scott Branden wrote: > > > > Hi Greg, > > > > > > > > I am now back from leave to continue this patch. Comment below. > > > > > > > > On 2019-05-23 10:22 p.m., Greg Kroah-Hartman wrote: > > > > > On Thu, May 23, 2019 at 10:01:38PM -0700, Scott Branden wrote: > > > > > > On 2019-05-23 9:54 a.m., Greg Kroah-Hartman wrote: > > > > > > > On Thu, May 23, 2019 at 09:36:02AM -0700, Scott Branden wrote: > > > > > > > > Hi Greg, > > > > > > > > > > > > > > > > On 2019-05-22 10:52 p.m., Greg Kroah-Hartman wrote: > > > > > > > > > On Wed, May 22, 2019 at 07:51:12PM -0700, Scott Branden wrote: > > > > > > > > > > Add offset to request_firmware_into_buf to allow for > > > > > > > > > > portions > > > > > > > > > > of firmware file to be read into a buffer. Necessary where > > > > > > > > > > firmware > > > > > > > > > > needs to be loaded in portions from file in memory > > > > > > > > > > constrained systems. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Scott Branden > > > > > > > > > > --- > > > > > > > > > > drivers/base/firmware_loader/firmware.h | 5 +++ > > > > > > > > > > drivers/base/firmware_loader/main.c | 49 > > > > > > > > > > + > > > > > > > > > > include/linux/firmware.h| 8 +++- > > > > > > > > > > 3 files changed, 45 insertions(+), 17 deletions(-) > > > > > > > > > No new firmware test for this new option? How do we know it > > > > > > > > > even works? > > > > > > > > I was unaware there are existing firmware tests. Please let me > > > > > > > > know where > > > > > > > > these tests exists and I can add a test for this new option. > > > > > > > tools/testing/selftests/firmware/ > > > > > > Unfortunately, there doesn't seem to be a test for the existing > > > > > > request_firmware_into_buf api. > > > > > Are you sure? The test is for userspace functionality, there isn't > > > > > kernel unit tests here. You need to verify that you didn't break > > > > > existing functionality as well as verify that your new functionality > > > > > works. > > > > I managed to figure out how to build and run > > > > tools/testing/selftest/firmware/fw_run_tests.sh > > > > > > > > and my changes don't break existing functionality. > > I'm soon going to release something that is going to let you do this > > faster and easier, let me know if you had troubles in trying to figure > > out how to not regress the kernel using this. > > Yes, I had troubles in trying to figure it out. The kernel build should > > create an entire initrd with all the necessary components in it for testing > purposes. > > And the firmware test will now take me some time to figure out how it all > works. > > Could you please explain what you are going to release soon? I don't want > to waste Sorry for the delay but I promise that I tried hard to get this out ASAP. https://github.com/mcgrof/fw-kdevops This now can be used to more easily let you start an environment to test the firmware API. Too late for you I gather, but perhaps others can take advantage. Luis
Re: [PATCH 2/3] firmware: add offset to request_firmware_into_buf
Hi Luis, On 2019-08-01 10:42 a.m., Luis Chamberlain wrote: On Thu, Aug 01, 2019 at 08:18:01AM +0200, Greg Kroah-Hartman wrote: On Wed, Jul 31, 2019 at 05:18:32PM -0700, Scott Branden wrote: Hi Greg, I am now back from leave to continue this patch. Comment below. On 2019-05-23 10:22 p.m., Greg Kroah-Hartman wrote: On Thu, May 23, 2019 at 10:01:38PM -0700, Scott Branden wrote: On 2019-05-23 9:54 a.m., Greg Kroah-Hartman wrote: On Thu, May 23, 2019 at 09:36:02AM -0700, Scott Branden wrote: Hi Greg, On 2019-05-22 10:52 p.m., Greg Kroah-Hartman wrote: On Wed, May 22, 2019 at 07:51:12PM -0700, Scott Branden wrote: Add offset to request_firmware_into_buf to allow for portions of firmware file to be read into a buffer. Necessary where firmware needs to be loaded in portions from file in memory constrained systems. Signed-off-by: Scott Branden --- drivers/base/firmware_loader/firmware.h | 5 +++ drivers/base/firmware_loader/main.c | 49 + include/linux/firmware.h| 8 +++- 3 files changed, 45 insertions(+), 17 deletions(-) No new firmware test for this new option? How do we know it even works? I was unaware there are existing firmware tests. Please let me know where these tests exists and I can add a test for this new option. tools/testing/selftests/firmware/ Unfortunately, there doesn't seem to be a test for the existing request_firmware_into_buf api. Are you sure? The test is for userspace functionality, there isn't kernel unit tests here. You need to verify that you didn't break existing functionality as well as verify that your new functionality works. I managed to figure out how to build and run tools/testing/selftest/firmware/fw_run_tests.sh and my changes don't break existing functionality. I'm soon going to release something that is going to let you do this faster and easier, let me know if you had troubles in trying to figure out how to not regress the kernel using this. Yes, I had troubles in trying to figure it out. The kernel build should create an entire initrd with all the necessary components in it for testing purposes. And the firmware test will now take me some time to figure out how it all works. Could you please explain what you are going to release soon? I don't want to waste my time getting something working if everything is going to change on me right away? But, I find no use of request_firmware_into_buf in lib/test_firmware.c (triggered by fw_run_tests.sh). Is there another test for request_firmware_into_buf? I have no idea, sorry. The folks who implemented request_firmware_into_buf() didn't add a respective test, because, well, this API went upstream IMO without much ACKs / review, and even no damn users. Now we have a user so we're stuck with it. The request_firmware_into_buf is a necessity for me as well (along with the need for a partial request of the file which I'm adding). So new testing calls for it would be appreciated. If you have questions I am happy to help. If you're an expert on the firmware test and can quickly add a simple test of request_firmware_into_buf it would be appreciated. If not, I'm going to have to dig further into this and send early versions of a test out which would be great for you to comment on. Luis Thanks, Scott
Re: [PATCH 2/3] firmware: add offset to request_firmware_into_buf
On Thu, Aug 01, 2019 at 08:18:01AM +0200, Greg Kroah-Hartman wrote: > On Wed, Jul 31, 2019 at 05:18:32PM -0700, Scott Branden wrote: > > Hi Greg, > > > > I am now back from leave to continue this patch. Comment below. > > > > On 2019-05-23 10:22 p.m., Greg Kroah-Hartman wrote: > > > On Thu, May 23, 2019 at 10:01:38PM -0700, Scott Branden wrote: > > > > On 2019-05-23 9:54 a.m., Greg Kroah-Hartman wrote: > > > > > On Thu, May 23, 2019 at 09:36:02AM -0700, Scott Branden wrote: > > > > > > Hi Greg, > > > > > > > > > > > > On 2019-05-22 10:52 p.m., Greg Kroah-Hartman wrote: > > > > > > > On Wed, May 22, 2019 at 07:51:12PM -0700, Scott Branden wrote: > > > > > > > > Add offset to request_firmware_into_buf to allow for portions > > > > > > > > of firmware file to be read into a buffer. Necessary where > > > > > > > > firmware > > > > > > > > needs to be loaded in portions from file in memory constrained > > > > > > > > systems. > > > > > > > > > > > > > > > > Signed-off-by: Scott Branden > > > > > > > > --- > > > > > > > > drivers/base/firmware_loader/firmware.h | 5 +++ > > > > > > > > drivers/base/firmware_loader/main.c | 49 > > > > > > > > + > > > > > > > > include/linux/firmware.h| 8 +++- > > > > > > > > 3 files changed, 45 insertions(+), 17 deletions(-) > > > > > > > No new firmware test for this new option? How do we know it even > > > > > > > works? > > > > > > I was unaware there are existing firmware tests. Please let me > > > > > > know where > > > > > > these tests exists and I can add a test for this new option. > > > > > tools/testing/selftests/firmware/ > > > > Unfortunately, there doesn't seem to be a test for the existing > > > > request_firmware_into_buf api. > > > Are you sure? The test is for userspace functionality, there isn't > > > kernel unit tests here. You need to verify that you didn't break > > > existing functionality as well as verify that your new functionality > > > works. > > > > I managed to figure out how to build and run > > tools/testing/selftest/firmware/fw_run_tests.sh > > > > and my changes don't break existing functionality. I'm soon going to release something that is going to let you do this faster and easier, let me know if you had troubles in trying to figure out how to not regress the kernel using this. > > But, I find no use of request_firmware_into_buf in lib/test_firmware.c > > (triggered by fw_run_tests.sh). > > > > Is there another test for request_firmware_into_buf? > > I have no idea, sorry. The folks who implemented request_firmware_into_buf() didn't add a respective test, because, well, this API went upstream IMO without much ACKs / review, and even no damn users. Now we have a user so we're stuck with it. So new testing calls for it would be appreciated. If you have questions I am happy to help. Luis
Re: [PATCH 2/3] firmware: add offset to request_firmware_into_buf
On Wed, Jul 31, 2019 at 05:18:32PM -0700, Scott Branden wrote: > Hi Greg, > > I am now back from leave to continue this patch. Comment below. > > On 2019-05-23 10:22 p.m., Greg Kroah-Hartman wrote: > > On Thu, May 23, 2019 at 10:01:38PM -0700, Scott Branden wrote: > > > On 2019-05-23 9:54 a.m., Greg Kroah-Hartman wrote: > > > > On Thu, May 23, 2019 at 09:36:02AM -0700, Scott Branden wrote: > > > > > Hi Greg, > > > > > > > > > > On 2019-05-22 10:52 p.m., Greg Kroah-Hartman wrote: > > > > > > On Wed, May 22, 2019 at 07:51:12PM -0700, Scott Branden wrote: > > > > > > > Add offset to request_firmware_into_buf to allow for portions > > > > > > > of firmware file to be read into a buffer. Necessary where > > > > > > > firmware > > > > > > > needs to be loaded in portions from file in memory constrained > > > > > > > systems. > > > > > > > > > > > > > > Signed-off-by: Scott Branden > > > > > > > --- > > > > > > > drivers/base/firmware_loader/firmware.h | 5 +++ > > > > > > > drivers/base/firmware_loader/main.c | 49 > > > > > > > + > > > > > > > include/linux/firmware.h| 8 +++- > > > > > > > 3 files changed, 45 insertions(+), 17 deletions(-) > > > > > > No new firmware test for this new option? How do we know it even > > > > > > works? > > > > > I was unaware there are existing firmware tests. Please let me know > > > > > where > > > > > these tests exists and I can add a test for this new option. > > > > tools/testing/selftests/firmware/ > > > Unfortunately, there doesn't seem to be a test for the existing > > > request_firmware_into_buf api. > > Are you sure? The test is for userspace functionality, there isn't > > kernel unit tests here. You need to verify that you didn't break > > existing functionality as well as verify that your new functionality > > works. > > I managed to figure out how to build and run > tools/testing/selftest/firmware/fw_run_tests.sh > > and my changes don't break existing functionality. > > But, I find no use of request_firmware_into_buf in lib/test_firmware.c > (triggered by fw_run_tests.sh). > > Is there another test for request_firmware_into_buf? I have no idea, sorry. greg k-h
Re: [PATCH 2/3] firmware: add offset to request_firmware_into_buf
Hi Greg, I am now back from leave to continue this patch. Comment below. On 2019-05-23 10:22 p.m., Greg Kroah-Hartman wrote: On Thu, May 23, 2019 at 10:01:38PM -0700, Scott Branden wrote: On 2019-05-23 9:54 a.m., Greg Kroah-Hartman wrote: On Thu, May 23, 2019 at 09:36:02AM -0700, Scott Branden wrote: Hi Greg, On 2019-05-22 10:52 p.m., Greg Kroah-Hartman wrote: On Wed, May 22, 2019 at 07:51:12PM -0700, Scott Branden wrote: Add offset to request_firmware_into_buf to allow for portions of firmware file to be read into a buffer. Necessary where firmware needs to be loaded in portions from file in memory constrained systems. Signed-off-by: Scott Branden --- drivers/base/firmware_loader/firmware.h | 5 +++ drivers/base/firmware_loader/main.c | 49 + include/linux/firmware.h| 8 +++- 3 files changed, 45 insertions(+), 17 deletions(-) No new firmware test for this new option? How do we know it even works? I was unaware there are existing firmware tests. Please let me know where these tests exists and I can add a test for this new option. tools/testing/selftests/firmware/ Unfortunately, there doesn't seem to be a test for the existing request_firmware_into_buf api. Are you sure? The test is for userspace functionality, there isn't kernel unit tests here. You need to verify that you didn't break existing functionality as well as verify that your new functionality works. I managed to figure out how to build and run tools/testing/selftest/firmware/fw_run_tests.sh and my changes don't break existing functionality. But, I find no use of request_firmware_into_buf in lib/test_firmware.c (triggered by fw_run_tests.sh). Is there another test for request_firmware_into_buf? We have tested this with a new driver in development which requires the firmware file to be read in portions into memory. I can add my tested-by and others to the commit message if desired. I can't take new apis without an in-kernel user, you all know this... OK, It will have to wait then as I was hoping to get this in before my leave. Throwing new code over the wall and running away is a sure way to ensure that your code will be ignored :) thanks, greg k-h
Re: [PATCH 2/3] firmware: add offset to request_firmware_into_buf
On Thu, May 23, 2019 at 10:01:38PM -0700, Scott Branden wrote: > > On 2019-05-23 9:54 a.m., Greg Kroah-Hartman wrote: > > On Thu, May 23, 2019 at 09:36:02AM -0700, Scott Branden wrote: > > > Hi Greg, > > > > > > On 2019-05-22 10:52 p.m., Greg Kroah-Hartman wrote: > > > > On Wed, May 22, 2019 at 07:51:12PM -0700, Scott Branden wrote: > > > > > Add offset to request_firmware_into_buf to allow for portions > > > > > of firmware file to be read into a buffer. Necessary where firmware > > > > > needs to be loaded in portions from file in memory constrained > > > > > systems. > > > > > > > > > > Signed-off-by: Scott Branden > > > > > --- > > > > >drivers/base/firmware_loader/firmware.h | 5 +++ > > > > >drivers/base/firmware_loader/main.c | 49 > > > > > + > > > > >include/linux/firmware.h| 8 +++- > > > > >3 files changed, 45 insertions(+), 17 deletions(-) > > > > No new firmware test for this new option? How do we know it even works? > > > I was unaware there are existing firmware tests. Please let me know where > > > these tests exists and I can add a test for this new option. > > tools/testing/selftests/firmware/ > > Unfortunately, there doesn't seem to be a test for the existing > request_firmware_into_buf api. Are you sure? The test is for userspace functionality, there isn't kernel unit tests here. You need to verify that you didn't break existing functionality as well as verify that your new functionality works. > > > We have tested this with a new driver in development which requires the > > > firmware file to be read in portions into memory. I can add my tested-by > > > and others to the commit message if desired. > > I can't take new apis without an in-kernel user, you all know this... > > OK, It will have to wait then as I was hoping to get this in before my > leave. Throwing new code over the wall and running away is a sure way to ensure that your code will be ignored :) thanks, greg k-h
Re: [PATCH 2/3] firmware: add offset to request_firmware_into_buf
On 2019-05-23 9:54 a.m., Greg Kroah-Hartman wrote: On Thu, May 23, 2019 at 09:36:02AM -0700, Scott Branden wrote: Hi Greg, On 2019-05-22 10:52 p.m., Greg Kroah-Hartman wrote: On Wed, May 22, 2019 at 07:51:12PM -0700, Scott Branden wrote: Add offset to request_firmware_into_buf to allow for portions of firmware file to be read into a buffer. Necessary where firmware needs to be loaded in portions from file in memory constrained systems. Signed-off-by: Scott Branden --- drivers/base/firmware_loader/firmware.h | 5 +++ drivers/base/firmware_loader/main.c | 49 + include/linux/firmware.h| 8 +++- 3 files changed, 45 insertions(+), 17 deletions(-) No new firmware test for this new option? How do we know it even works? I was unaware there are existing firmware tests. Please let me know where these tests exists and I can add a test for this new option. tools/testing/selftests/firmware/ Unfortunately, there doesn't seem to be a test for the existing request_firmware_into_buf api. Looks like it will be more work that I thought enhancing a test that doesn't exist. We have tested this with a new driver in development which requires the firmware file to be read in portions into memory. I can add my tested-by and others to the commit message if desired. I can't take new apis without an in-kernel user, you all know this... OK, It will have to wait then as I was hoping to get this in before my leave. But adding a selftest and upstreaming the necessary driver won't be possible for a few months now. thanks, greg k-h Thanks for explaining the requirements. Scott
Re: [PATCH 2/3] firmware: add offset to request_firmware_into_buf
On Thu, May 23, 2019 at 09:36:02AM -0700, Scott Branden wrote: > Hi Greg, > > On 2019-05-22 10:52 p.m., Greg Kroah-Hartman wrote: > > On Wed, May 22, 2019 at 07:51:12PM -0700, Scott Branden wrote: > > > Add offset to request_firmware_into_buf to allow for portions > > > of firmware file to be read into a buffer. Necessary where firmware > > > needs to be loaded in portions from file in memory constrained systems. > > > > > > Signed-off-by: Scott Branden > > > --- > > > drivers/base/firmware_loader/firmware.h | 5 +++ > > > drivers/base/firmware_loader/main.c | 49 + > > > include/linux/firmware.h| 8 +++- > > > 3 files changed, 45 insertions(+), 17 deletions(-) > > No new firmware test for this new option? How do we know it even works? > > I was unaware there are existing firmware tests. Please let me know where > these tests exists and I can add a test for this new option. tools/testing/selftests/firmware/ > We have tested this with a new driver in development which requires the > firmware file to be read in portions into memory. I can add my tested-by > and others to the commit message if desired. I can't take new apis without an in-kernel user, you all know this... thanks, greg k-h
Re: [PATCH 2/3] firmware: add offset to request_firmware_into_buf
Hi Greg, On 2019-05-22 10:52 p.m., Greg Kroah-Hartman wrote: On Wed, May 22, 2019 at 07:51:12PM -0700, Scott Branden wrote: Add offset to request_firmware_into_buf to allow for portions of firmware file to be read into a buffer. Necessary where firmware needs to be loaded in portions from file in memory constrained systems. Signed-off-by: Scott Branden --- drivers/base/firmware_loader/firmware.h | 5 +++ drivers/base/firmware_loader/main.c | 49 + include/linux/firmware.h| 8 +++- 3 files changed, 45 insertions(+), 17 deletions(-) No new firmware test for this new option? How do we know it even works? I was unaware there are existing firmware tests. Please let me know where these tests exists and I can add a test for this new option. We have tested this with a new driver in development which requires the firmware file to be read in portions into memory. I can add my tested-by and others to the commit message if desired. :) thanks, greg k-h Regards, Scott
Re: [PATCH 2/3] firmware: add offset to request_firmware_into_buf
On Wed, May 22, 2019 at 07:51:12PM -0700, Scott Branden wrote: > Add offset to request_firmware_into_buf to allow for portions > of firmware file to be read into a buffer. Necessary where firmware > needs to be loaded in portions from file in memory constrained systems. > > Signed-off-by: Scott Branden > --- > drivers/base/firmware_loader/firmware.h | 5 +++ > drivers/base/firmware_loader/main.c | 49 + > include/linux/firmware.h| 8 +++- > 3 files changed, 45 insertions(+), 17 deletions(-) No new firmware test for this new option? How do we know it even works? :) thanks, greg k-h
[PATCH 2/3] firmware: add offset to request_firmware_into_buf
Add offset to request_firmware_into_buf to allow for portions of firmware file to be read into a buffer. Necessary where firmware needs to be loaded in portions from file in memory constrained systems. Signed-off-by: Scott Branden --- drivers/base/firmware_loader/firmware.h | 5 +++ drivers/base/firmware_loader/main.c | 49 + include/linux/firmware.h| 8 +++- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h index 4c1395f8e7ed..d73d400c2023 100644 --- a/drivers/base/firmware_loader/firmware.h +++ b/drivers/base/firmware_loader/firmware.h @@ -29,6 +29,8 @@ * firmware caching mechanism. * @FW_OPT_NOFALLBACK: Disable the fallback mechanism. Takes precedence over * _OPT_UEVENT and _OPT_USERHELPER. + * @FW_OPT_PARTIAL: Allow partial read of firmware instead of needing to read + * entire file. */ enum fw_opt { FW_OPT_UEVENT = BIT(0), @@ -37,6 +39,7 @@ enum fw_opt { FW_OPT_NO_WARN =BIT(3), FW_OPT_NOCACHE =BIT(4), FW_OPT_NOFALLBACK = BIT(5), + FW_OPT_PARTIAL =BIT(6), }; enum fw_status { @@ -64,6 +67,8 @@ struct fw_priv { void *data; size_t size; size_t allocated_size; + size_t offset; + unsigned int flags; #ifdef CONFIG_FW_LOADER_USER_HELPER bool is_paged_buf; bool need_uevent; diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 7eaaf5ee5ba6..34d4f043b7c8 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -166,7 +166,8 @@ static int fw_cache_piggyback_on_request(const char *name); static struct fw_priv *__allocate_fw_priv(const char *fw_name, struct firmware_cache *fwc, - void *dbuf, size_t size) + void *dbuf, size_t size, + size_t offset, unsigned int flags) { struct fw_priv *fw_priv; @@ -184,6 +185,8 @@ static struct fw_priv *__allocate_fw_priv(const char *fw_name, fw_priv->fwc = fwc; fw_priv->data = dbuf; fw_priv->allocated_size = size; + fw_priv->offset = offset; + fw_priv->flags = flags; fw_state_init(fw_priv); #ifdef CONFIG_FW_LOADER_USER_HELPER INIT_LIST_HEAD(_priv->pending_list); @@ -209,9 +212,11 @@ static struct fw_priv *__lookup_fw_priv(const char *fw_name) static int alloc_lookup_fw_priv(const char *fw_name, struct firmware_cache *fwc, struct fw_priv **fw_priv, void *dbuf, - size_t size, enum fw_opt opt_flags) + size_t size, enum fw_opt opt_flags, + size_t offset) { struct fw_priv *tmp; + unsigned int pread_flags; spin_lock(>lock); if (!(opt_flags & FW_OPT_NOCACHE)) { @@ -225,7 +230,12 @@ static int alloc_lookup_fw_priv(const char *fw_name, } } - tmp = __allocate_fw_priv(fw_name, fwc, dbuf, size); + if (opt_flags & FW_OPT_PARTIAL) + pread_flags = KERNEL_PREAD_FLAG_PART; + else + pread_flags = KERNEL_PREAD_FLAG_WHOLE; + + tmp = __allocate_fw_priv(fw_name, fwc, dbuf, size, offset, pread_flags); if (tmp) { INIT_LIST_HEAD(>list); if (!(opt_flags & FW_OPT_NOCACHE)) @@ -325,8 +335,9 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv) } fw_priv->size = 0; - rc = kernel_read_file_from_path(path, _priv->data, , - msize, id); + rc = kernel_pread_file_from_path(path, _priv->data, , +fw_priv->offset, msize, +fw_priv->flags, id); if (rc) { if (rc != -ENOENT) dev_warn(device, "loading %s failed with error %d\n", @@ -500,7 +511,7 @@ int assign_fw(struct firmware *fw, struct device *device, static int _request_firmware_prepare(struct firmware **firmware_p, const char *name, struct device *device, void *dbuf, size_t size, - enum fw_opt opt_flags) + enum fw_opt opt_flags, size_t offset) { struct firmware *firmware; struct fw_priv *fw_priv; @@ -519,7 +530,7 @@ _request_firmware_prepare(struct firmware **firmware_p, const char *name, } ret = alloc_lookup_fw_priv(name, _cache, _priv, dbuf, size, - opt_flags); + opt_flags, offset); /*