6 comments:
File src/drivers/smmstore/store.c:
Patch Set #13, Line 60: rdev.region
*region_device_region(&rdev);
Patch Set #13, Line 87: store_rdev
This change seems like quite the short cut. store_rdev only is used once below. However, the comment I made on patchset 12 on line 106 equally applies still as you have violated the conditions needed to use the API.
Patch Set #13, Line 118: int smmstore_read_region(void *buf, ssize_t *bufsize)
There are 3 APIs to SMM store from the interface:
1. smmstore_read_region()
2. smmstore_append_data()
3. smmstore_clear_region()
All 3 need to determine the location of the region itself. They are all one-shot and expect no runtime state to persist this compliation unit outside of those calls.
#1 only needs read access
#2 needs read and write access
#3 only needs write access
If you want one function to rule them all then you will need to construct the incoherent_rdev correctly. The code below should work the way you want:
static int lookup_store(struct region_device *rstore)
{
static struct region_device read_rdev, write_rdev;
static struct incoherent_rdev store_irdev;
struct region region;
struct region_device *rdev;
if (lookup_store_region(®ion))
return -1;
if (boot_device_ro_subregion(®ion, &read_rdev) < 0)
return -1;
if (boot_device_rw_subregion(®ion, &write_rdev) < 0)
return -1;
rdev = incoherent_rdev_init(&store_irdev, ®ion, &read_rdev, &write_rdev);
if (rdev == NULL)
return -1;
return rdev_chain(rstore, rdev, 0, region_device_sz(rdev));
}
Patch Set #13, Line 211: data_sz
I don't understand why one is passing in data_sz scan_end() can obtain it directly since 'store' is being passed in. And since end is onl used once, by this function, you can just adjust store within scan_end() to have store only point to the remaining region. Then below you would rdev_chain(..., 0, size) for a bounds check.
Patch Set #13, Line 219: &store) < 0) {
You already have a writable rdev by way of using incoherent_rdev in lookup_store().
This should be 0. The rdev_chain() tracks the window of the region it is operating on. As such the offsets are relative to that: (0, size]. Therefore end needs to be reset to 0 to start the writes.
To view, visit change 30432. To unsubscribe, or for help writing mail filters, visit settings.