Re: [PATCH V10] Add support for BTRFS raid5/6 to GRUB

2018-10-22 Thread Nick Terrell



> On Oct 22, 2018, at 4:02 AM, Daniel Kiper  wrote:
> 
> On Thu, Oct 18, 2018 at 07:55:32PM +0200, Goffredo Baroncelli wrote:
>> 
>> Hi All,
>> 
>> the aim of this patches set is to provide support for a BTRFS raid5/6
>> filesystem in GRUB.
>> 
>> The first patch, implements the basic support for raid5/6. I.e this works 
>> when
>> all the disks are present.
>> 
>> The next 5 patches, are preparatory ones.
>> 
>> The 7th patch implements the raid5 recovery for btrfs (i.e. handling the
>> disappearing of 1 disk).
>> The 8th patch makes the code for handling the raid6 recovery more generic.
>> The last one implements the raid6 recovery for btrfs (i.e. handling the
>> disappearing up to two disks).
>> 
>> I tested the code in grub-emu, and it works both with all the disks,
>> and with some disks missing. I checked the crc32 calculated from grub and
>> from linux and these matched. Finally I checked if the support for md raid6
>> still works properly, and it does (with all drives and with up to 2 drives
>> missing)
>> 
>> Comments are welcome.
> 
> In general whole patch series LGTM. +/- some nit picks including changes
> for patch #7. If you are OK with them and there are no objections then
> I will apply the patches in a week or so.

Awesome! I'll look for the update and send an rebased version of the zstd
patch set when it is out.

-Nick
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 1/9] btrfs: Add support for reading a filesystem with a RAID 5 or RAID 6 profile.

2018-10-22 Thread Goffredo Baroncelli
From: Goffredo Baroncelli 

Signed-off-by: Goffredo Baroncelli 
Signed-off-by: Daniel Kiper 
Reviewed-by: Daniel Kiper 
---
 grub-core/fs/btrfs.c | 73 
 1 file changed, 73 insertions(+)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index be195448d..9122169aa 100644
--- a/grub-core/fs/btrfs.c
+++ b/grub-core/fs/btrfs.c
@@ -119,6 +119,8 @@ struct grub_btrfs_chunk_item
 #define GRUB_BTRFS_CHUNK_TYPE_RAID1 0x10
 #define GRUB_BTRFS_CHUNK_TYPE_DUPLICATED0x20
 #define GRUB_BTRFS_CHUNK_TYPE_RAID100x40
+#define GRUB_BTRFS_CHUNK_TYPE_RAID5 0x80
+#define GRUB_BTRFS_CHUNK_TYPE_RAID6 0x100
   grub_uint8_t dummy2[0xc];
   grub_uint16_t nstripes;
   grub_uint16_t nsubstripes;
@@ -764,6 +766,77 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data, 
grub_disk_addr_t addr,
  stripe_offset = low + chunk_stripe_length
* high;
  csize = chunk_stripe_length - low;
+ break;
+   }
+ case GRUB_BTRFS_CHUNK_TYPE_RAID5:
+ case GRUB_BTRFS_CHUNK_TYPE_RAID6:
+   {
+ grub_uint64_t nparities, stripe_nr, high, low;
+
+ redundancy = 1;   /* no redundancy for now */
+
+ if (grub_le_to_cpu64 (chunk->type) & GRUB_BTRFS_CHUNK_TYPE_RAID5)
+   {
+ grub_dprintf ("btrfs", "RAID5\n");
+ nparities = 1;
+   }
+ else
+   {
+ grub_dprintf ("btrfs", "RAID6\n");
+ nparities = 2;
+   }
+
+ /*
+  * RAID 6 layout consists of several stripes spread over
+  * the disks, e.g.:
+  *
+  *   Disk_0  Disk_1  Disk_2  Disk_3
+  * A0  B0  P0  Q0
+  * Q1  A1  B1  P1
+  * P2  Q2  A2  B2
+  *
+  * Note: placement of the parities depend on row number.
+  *
+  * Pay attention that the btrfs terminology may differ from
+  * terminology used in other RAID implementations, e.g. LVM,
+  * dm or md. The main difference is that btrfs calls contiguous
+  * block of data on a given disk, e.g. A0, stripe instead of 
chunk.
+  *
+  * The variables listed below have following meaning:
+  *   - stripe_nr is the stripe number excluding the parities
+  * (A0 = 0, B0 = 1, A1 = 2, B1 = 3, etc.),
+  *   - high is the row number (0 for A0...Q0, 1 for Q1...P1, 
etc.),
+  *   - stripen is the disk number in a row (0 for A0, Q1, P2,
+  * 1 for B0, A1, Q2, etc.),
+  *   - off is the logical address to read,
+  *   - chunk_stripe_length is the size of a stripe (typically 64 
KiB),
+  *   - nstripes is the number of disks in a row,
+  *   - low is the offset of the data inside a stripe,
+  *   - stripe_offset is the data offset in an array,
+  *   - csize is the "potential" data to read; it will be reduced
+  * to size if the latter is smaller,
+  *   - nparities is the number of parities (1 for RAID 5, 2 for
+  * RAID 6); used only in RAID 5/6 code.
+  */
+ stripe_nr = grub_divmod64 (off, chunk_stripe_length, );
+
+ /*
+  * stripen is computed without the parities
+  * (0 for A0, A1, A2, 1 for B0, B1, B2, etc.).
+  */
+ high = grub_divmod64 (stripe_nr, nstripes - nparities, );
+
+ /*
+  * The stripes are spread over the disks. Every each row their
+  * positions are shifted by 1 place. So, the real disks number
+  * change. Hence, we have to take into account current row number
+  * modulo nstripes (0 for A0, 1 for A1, 2 for A2, etc.).
+  */
+ grub_divmod64 (high + stripen, nstripes, );
+
+ stripe_offset = low + chunk_stripe_length * high;
+ csize = chunk_stripe_length - low;
+
  break;
}
  default:
-- 
2.19.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 7/9] btrfs: Add support for recovery for a RAID 5 btrfs profiles.

2018-10-22 Thread Goffredo Baroncelli
From: Goffredo Baroncelli 

Add support for recovery for a RAID 5 btrfs profile. In addition
it is added some code as preparatory work for RAID 6 recovery code.

Signed-off-by: Goffredo Baroncelli 
Reviewed-by: Daniel Kiper 
---
 grub-core/fs/btrfs.c | 159 +--
 1 file changed, 154 insertions(+), 5 deletions(-)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index ea97f0502..965888da4 100644
--- a/grub-core/fs/btrfs.c
+++ b/grub-core/fs/btrfs.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
@@ -665,6 +666,138 @@ btrfs_read_from_chunk (struct grub_btrfs_data *data,
 return err;
 }
 
+struct raid56_buffer {
+  void *buf;
+  int  data_is_valid;
+};
+
+static void
+rebuild_raid5 (char *dest, struct raid56_buffer *buffers,
+  grub_uint64_t nstripes, grub_uint64_t csize)
+{
+  grub_uint64_t i;
+  int first;
+
+  for(i = 0; buffers[i].data_is_valid && i < nstripes; i++);
+
+  if (i == nstripes)
+{
+  grub_dprintf ("btrfs", "called rebuild_raid5(), but all disks are OK\n");
+  return;
+}
+
+  grub_dprintf ("btrfs", "rebuilding RAID 5 stripe #%" PRIuGRUB_UINT64_T "\n", 
i);
+
+  for (i = 0, first = 1; i < nstripes; i++)
+{
+  if (!buffers[i].data_is_valid)
+   continue;
+
+  if (first) {
+   grub_memcpy(dest, buffers[i].buf, csize);
+   first = 0;
+  } else
+   grub_crypto_xor (dest, dest, buffers[i].buf, csize);
+}
+}
+
+static grub_err_t
+raid56_read_retry (struct grub_btrfs_data *data,
+  struct grub_btrfs_chunk_item *chunk,
+  grub_uint64_t stripe_offset,
+  grub_uint64_t csize, void *buf)
+{
+  struct raid56_buffer *buffers;
+  grub_uint64_t nstripes = grub_le_to_cpu16 (chunk->nstripes);
+  grub_uint64_t chunk_type = grub_le_to_cpu64 (chunk->type);
+  grub_err_t ret = GRUB_ERR_OUT_OF_MEMORY;
+  grub_uint64_t i, failed_devices;
+
+  buffers = grub_zalloc (sizeof(*buffers) * nstripes);
+  if (!buffers)
+goto cleanup;
+
+  for (i = 0; i < nstripes; i++)
+{
+  buffers[i].buf = grub_zalloc (csize);
+  if (!buffers[i].buf)
+   goto cleanup;
+}
+
+  for (failed_devices = 0, i = 0; i < nstripes; i++)
+{
+  struct grub_btrfs_chunk_stripe *stripe;
+  grub_disk_addr_t paddr;
+  grub_device_t dev;
+  grub_err_t err;
+
+  /* The struct grub_btrfs_chunk_stripe array lives behind struct
+grub_btrfs_chunk_item. */
+  stripe = (struct grub_btrfs_chunk_stripe *) (chunk + 1) + i;
+
+  paddr = grub_le_to_cpu64 (stripe->offset) + stripe_offset;
+  grub_dprintf ("btrfs", "reading paddr %" PRIxGRUB_UINT64_T
+" from stripe ID %" PRIxGRUB_UINT64_T "\n", paddr,
+stripe->device_id);
+
+  dev = find_device (data, stripe->device_id);
+  if (!dev)
+   {
+ grub_dprintf ("btrfs", "stripe %" PRIuGRUB_UINT64_T " FAILED (dev ID 
%"
+   PRIxGRUB_UINT64_T ")\n", i, stripe->device_id);
+ failed_devices++;
+ continue;
+   }
+
+  err = grub_disk_read (dev->disk, paddr >> GRUB_DISK_SECTOR_BITS,
+   paddr & (GRUB_DISK_SECTOR_SIZE - 1),
+   csize, buffers[i].buf);
+  if (err == GRUB_ERR_NONE)
+   {
+ buffers[i].data_is_valid = 1;
+ grub_dprintf ("btrfs", "stripe %" PRIuGRUB_UINT64_T " OK (dev ID %"
+   PRIxGRUB_UINT64_T ")\n", i, stripe->device_id);
+   }
+  else
+   {
+ grub_dprintf ("btrfs", "stripe %" PRIuGRUB_UINT64_T
+   " READ FAILED (dev ID %" PRIxGRUB_UINT64_T ")\n", i,
+   stripe->device_id);
+ failed_devices++;
+   }
+}
+
+  if (failed_devices > 1 && (chunk_type & GRUB_BTRFS_CHUNK_TYPE_RAID5))
+{
+  grub_dprintf ("btrfs",
+   "not enough disks for RAID 5: total %" PRIuGRUB_UINT64_T
+   ", missing %" PRIuGRUB_UINT64_T "\n",
+   nstripes, failed_devices);
+  ret = GRUB_ERR_READ_ERROR;
+  goto cleanup;
+}
+  else
+grub_dprintf ("btrfs",
+ "enough disks for RAID 5: total %"
+ PRIuGRUB_UINT64_T ", missing %" PRIuGRUB_UINT64_T "\n",
+ nstripes, failed_devices);
+
+  /* We have enough disks. So, rebuild the data. */
+  if (chunk_type & GRUB_BTRFS_CHUNK_TYPE_RAID5)
+rebuild_raid5 (buf, buffers, nstripes, csize);
+  else
+grub_dprintf ("btrfs", "called rebuild_raid6(), NOT IMPLEMENTED\n");
+
+  ret = GRUB_ERR_NONE;
+ cleanup:
+  if (buffers)
+for (i = 0; i < nstripes; i++)
+   grub_free (buffers[i].buf);
+  grub_free (buffers);
+
+  return ret;
+}
+
 static grub_err_t
 grub_btrfs_read_logical (struct grub_btrfs_data *data, grub_disk_addr_t addr,
 void *buf, grub_size_t size, int recursion_depth)
@@ -742,6 +875,10 @@ grub_btrfs_read_logical (struct 

[PATCH 6/9] btrfs: Refactor the code that read from disk

2018-10-22 Thread Goffredo Baroncelli
From: Goffredo Baroncelli 

Move the code in charge to read the data from disk into a separate
function. This helps to separate the error handling logic (which depends on
the different raid profiles) from the read from disk logic.
Refactoring this code increases the general readability too.

This is a preparatory patch, to help the adding of the RAID 5/6 recovery
code.

Signed-off-by: Goffredo Baroncelli 
Reviewed-by: Daniel Kiper 
---
 grub-core/fs/btrfs.c | 75 ++--
 1 file changed, 44 insertions(+), 31 deletions(-)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index dde0edd03..ea97f0502 100644
--- a/grub-core/fs/btrfs.c
+++ b/grub-core/fs/btrfs.c
@@ -625,6 +625,46 @@ find_device (struct grub_btrfs_data *data, grub_uint64_t 
id)
   return ctx.dev_found;
 }
 
+static grub_err_t
+btrfs_read_from_chunk (struct grub_btrfs_data *data,
+  struct grub_btrfs_chunk_item *chunk,
+  grub_uint64_t stripen, grub_uint64_t stripe_offset,
+  int redundancy, grub_uint64_t csize,
+  void *buf)
+{
+struct grub_btrfs_chunk_stripe *stripe;
+grub_disk_addr_t paddr;
+grub_device_t dev;
+grub_err_t err;
+
+stripe = (struct grub_btrfs_chunk_stripe *) (chunk + 1);
+/* Right now the redundancy handling is easy.
+   With RAID5-like it will be more difficult.  */
+stripe += stripen + redundancy;
+
+paddr = grub_le_to_cpu64 (stripe->offset) + stripe_offset;
+
+grub_dprintf ("btrfs", "stripe %" PRIxGRUB_UINT64_T
+ " maps to 0x%" PRIxGRUB_UINT64_T
+ ". Reading paddr 0x%" PRIxGRUB_UINT64_T "\n",
+ stripen, stripe->offset, paddr);
+
+dev = find_device (data, stripe->device_id);
+if (!dev)
+  {
+   grub_dprintf ("btrfs",
+ "couldn't find a necessary member device "
+ "of multi-device filesystem\n");
+   grub_errno = GRUB_ERR_NONE;
+   return GRUB_ERR_READ_ERROR;
+  }
+
+err = grub_disk_read (dev->disk, paddr >> GRUB_DISK_SECTOR_BITS,
+ paddr & (GRUB_DISK_SECTOR_SIZE - 1),
+ csize, buf);
+return err;
+}
+
 static grub_err_t
 grub_btrfs_read_logical (struct grub_btrfs_data *data, grub_disk_addr_t addr,
 void *buf, grub_size_t size, int recursion_depth)
@@ -638,7 +678,6 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data, 
grub_disk_addr_t addr,
   grub_err_t err = 0;
   struct grub_btrfs_key key_out;
   int challoc = 0;
-  grub_device_t dev;
   struct grub_btrfs_key key_in;
   grub_size_t chsize;
   grub_disk_addr_t chaddr;
@@ -884,36 +923,10 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data, 
grub_disk_addr_t addr,
 
for (i = 0; i < redundancy; i++)
  {
-   struct grub_btrfs_chunk_stripe *stripe;
-   grub_disk_addr_t paddr;
-
-   stripe = (struct grub_btrfs_chunk_stripe *) (chunk + 1);
-   /* Right now the redundancy handling is easy.
-  With RAID5-like it will be more difficult.  */
-   stripe += stripen + i;
-
-   paddr = grub_le_to_cpu64 (stripe->offset) + stripe_offset;
-
-   grub_dprintf ("btrfs", "stripe %" PRIxGRUB_UINT64_T
- " maps to 0x%" PRIxGRUB_UINT64_T "\n",
- stripen, stripe->offset);
-   grub_dprintf ("btrfs", "reading paddr 0x%" PRIxGRUB_UINT64_T
- "\n", paddr);
-
-   dev = find_device (data, stripe->device_id);
-   if (!dev)
- {
-   grub_dprintf ("btrfs",
- "couldn't find a necessary member device "
- "of multi-device filesystem\n");
-   err = grub_errno;
-   grub_errno = GRUB_ERR_NONE;
-   continue;
- }
-
-   err = grub_disk_read (dev->disk, paddr >> GRUB_DISK_SECTOR_BITS,
- paddr & (GRUB_DISK_SECTOR_SIZE - 1),
- csize, buf);
+   err = btrfs_read_from_chunk (data, chunk, stripen,
+stripe_offset,
+i, /* redundancy */
+csize, buf);
if (!err)
  break;
grub_errno = GRUB_ERR_NONE;
-- 
2.19.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 3/9] btrfs: Move the error logging from find_device() to its caller.

2018-10-22 Thread Goffredo Baroncelli
From: Goffredo Baroncelli 

The caller knows better if this error is fatal or not, i.e. another disk is
available or not.

This is a preparatory patch.

Signed-off-by: Goffredo Baroncelli 
Reviewed-by: Daniel Kiper 
---
 grub-core/fs/btrfs.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index 0cbf3551a..6b6e91cd1 100644
--- a/grub-core/fs/btrfs.c
+++ b/grub-core/fs/btrfs.c
@@ -603,12 +603,7 @@ find_device (struct grub_btrfs_data *data, grub_uint64_t 
id, int do_rescan)
   if (do_rescan)
 grub_device_iterate (find_device_iter, );
   if (!ctx.dev_found)
-{
-  grub_error (GRUB_ERR_BAD_FS,
- N_("couldn't find a necessary member device "
-"of multi-device filesystem"));
-  return NULL;
-}
+return NULL;
   data->n_devices_attached++;
   if (data->n_devices_attached > data->n_devices_allocated)
 {
@@ -905,6 +900,9 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data, 
grub_disk_addr_t addr,
dev = find_device (data, stripe->device_id, j);
if (!dev)
  {
+   grub_dprintf ("btrfs",
+ "couldn't find a necessary member device "
+ "of multi-device filesystem\n");
err = grub_errno;
grub_errno = GRUB_ERR_NONE;
continue;
-- 
2.19.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH 4/9] btrfs: Avoid a rescan for a device which was already not found.

2018-10-22 Thread Goffredo Baroncelli
From: Goffredo Baroncelli 

Currently read from missing device triggers rescan. However, it is never
recorded that the device is missing. So, each read of a missing device
triggers rescan again and again. This behavior causes a lot of unneeded
rescans leading to huge slowdowns.

This patch fixes above mentioned issue. Information about missing devices
is stored in the data->devices_attached[] array as NULL value in dev
member. Rescan is triggered only if no information is found for a given
device. This means that only first time read triggers rescan.

The patch drops premature return. This way data->devices_attached[] is
filled even when a given device is missing.

Signed-off-by: Goffredo Baroncelli 
Signed-off-by: Daniel Kiper 
Reviewed-by: Daniel Kiper 
---
 grub-core/fs/btrfs.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
index 6b6e91cd1..81f3bc120 100644
--- a/grub-core/fs/btrfs.c
+++ b/grub-core/fs/btrfs.c
@@ -588,7 +588,7 @@ find_device_iter (const char *name, void *data)
 }
 
 static grub_device_t
-find_device (struct grub_btrfs_data *data, grub_uint64_t id, int do_rescan)
+find_device (struct grub_btrfs_data *data, grub_uint64_t id)
 {
   struct find_device_ctx ctx = {
 .data = data,
@@ -600,10 +600,9 @@ find_device (struct grub_btrfs_data *data, grub_uint64_t 
id, int do_rescan)
   for (i = 0; i < data->n_devices_attached; i++)
 if (id == data->devices_attached[i].id)
   return data->devices_attached[i].dev;
-  if (do_rescan)
-grub_device_iterate (find_device_iter, );
-  if (!ctx.dev_found)
-return NULL;
+
+  grub_device_iterate (find_device_iter, );
+
   data->n_devices_attached++;
   if (data->n_devices_attached > data->n_devices_allocated)
 {
@@ -615,7 +614,8 @@ find_device (struct grub_btrfs_data *data, grub_uint64_t 
id, int do_rescan)
* sizeof (data->devices_attached[0]));
   if (!data->devices_attached)
{
- grub_device_close (ctx.dev_found);
+ if (ctx.dev_found)
+   grub_device_close (ctx.dev_found);
  data->devices_attached = tmp;
  return NULL;
}
@@ -897,7 +897,7 @@ grub_btrfs_read_logical (struct grub_btrfs_data *data, 
grub_disk_addr_t addr,
  " for laddr 0x%" PRIxGRUB_UINT64_T "\n", paddr,
  addr);
 
-   dev = find_device (data, stripe->device_id, j);
+   dev = find_device (data, stripe->device_id);
if (!dev)
  {
grub_dprintf ("btrfs",
@@ -974,7 +974,8 @@ grub_btrfs_unmount (struct grub_btrfs_data *data)
   unsigned i;
   /* The device 0 is closed one layer upper.  */
   for (i = 1; i < data->n_devices_attached; i++)
-grub_device_close (data->devices_attached[i].dev);
+if (data->devices_attached[i].dev)
+grub_device_close (data->devices_attached[i].dev);
   grub_free (data->devices_attached);
   grub_free (data->extent);
   grub_free (data);
-- 
2.19.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH V10] Add support for BTRFS raid5/6 to GRUB

2018-10-22 Thread Goffredo Baroncelli
On 22/10/2018 13.02, Daniel Kiper wrote:
> On Thu, Oct 18, 2018 at 07:55:32PM +0200, Goffredo Baroncelli wrote:
>>
>> Hi All,
>>
>> the aim of this patches set is to provide support for a BTRFS raid5/6
>> filesystem in GRUB.
[...]
> 
> In general whole patch series LGTM. +/- some nit picks including changes
> for patch #7. If you are OK with them and there are no objections then
> I will apply the patches in a week or so.

Good news; I will update the patch 7 and I will send patches set v11


BR
G.Baroncelli

> 
> Thank you for doing the work.
> 
> Daniel
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli 
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 13/18] xen: init memory regions for PVH

2018-10-22 Thread Daniel Kiper
On Mon, Oct 22, 2018 at 01:43:53PM +0200, Juergen Gross wrote:
> On 22/10/2018 13:31, Daniel Kiper wrote:
> > On Tue, Oct 09, 2018 at 01:03:12PM +0200, Juergen Gross wrote:
> >> Add all usable memory regions to grub memory management and add the
> >> needed mmap iterate code.
> >
> > I am missing a few words why this patch is needed. Especially why
> > grub_machine_mmap_iterate() has to belong to this patch. However,
> > I think that it should be introduced by patch in which
> > grub_machine_mmap_iterate() is used at some point.
>
> That would again lead to one giant PVH patch which you didn't like.
>
> grub_machine_mmap_iterate() is being used by grub common code like
> grub-core/lib/relocator.c or grub-core/mmap/mmap.c
>
> grub_machine_mmap_iterate() belongs into this patch as it is the
> main user of the memory map introduced here.

OK, let's leave it here. Though commit message has to be updated accordingly.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 18/18] xenpvh: add support to configure

2018-10-22 Thread Daniel Kiper
On Tue, Oct 09, 2018 at 01:03:17PM +0200, Juergen Gross wrote:
> Support platform i386/xenpvh in configure.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 

+/- XENPVH/xenpvh play still Reviewed-by: Daniel Kiper 

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 17/18] xenpvh: support grub-install for xenpvh

2018-10-22 Thread Daniel Kiper
On Tue, Oct 09, 2018 at 01:03:16PM +0200, Juergen Gross wrote:
> Add xenpvh support to grub-install.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 

+/- XENPVH play still Reviewed-by: Daniel Kiper 

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 16/18] xenpvh: support building a standalone image

2018-10-22 Thread Daniel Kiper
On Tue, Oct 09, 2018 at 01:03:15PM +0200, Juergen Gross wrote:
> Support mkimage for xenpvh.
>
> In order to avoid using plain integers for the ELF notes use the
> available Xen include instead. While at it replace the plain numbers
> for Xen PV mode, too.
>
> Signed-off-by: Juergen Gross 

+/- XENPVH/xenpvh play Reviewed-by: Daniel Kiper 

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 14/18] xenpvh: add build runes for grub-core

2018-10-22 Thread Juergen Gross
On 22/10/2018 13:41, Daniel Kiper wrote:
> On Tue, Oct 09, 2018 at 01:03:13PM +0200, Juergen Gross wrote:
>> Add the modifications to the build system needed to build a xenpvh
>> grub.
>>
>> Signed-off-by: Juergen Gross 
>> Reviewed-by: Daniel Kiper 
>> ---
>>  gentpl.py   |  4 ++--
>>  grub-core/Makefile.am   | 12 
>>  grub-core/Makefile.core.def | 35 +++
>>  3 files changed, 49 insertions(+), 2 deletions(-)
>>
>> diff --git a/gentpl.py b/gentpl.py
>> index da67965a4..9732b4aee 100644
>> --- a/gentpl.py
>> +++ b/gentpl.py
>> @@ -28,7 +28,7 @@ import re
>>
>>  GRUB_PLATFORMS = [ "emu", "i386_pc", "i386_efi", "i386_qemu", 
>> "i386_coreboot",
>> "i386_multiboot", "i386_ieee1275", "x86_64_efi",
>> -   "i386_xen", "x86_64_xen",
>> +   "i386_xen", "x86_64_xen", "i386_xenpvh",
> 
> s/i386_xenpvh/i386_xen_pvh/ here and below please...

... which is the answer to the question I had to your request for patch
07. I'll change it accordingly.

> 
>> "mips_loongson", "sparc64_ieee1275",
>> "powerpc_ieee1275", "mips_arc", "ia64_efi",
>> "mips_qemu_mips", "arm_uboot", "arm_efi", "arm64_efi",
>> @@ -71,7 +71,7 @@ GROUPS["videomodules"]   = GRUB_PLATFORMS[:];
>>  for i in GROUPS["videoinkernel"]: GROUPS["videomodules"].remove(i)
>>
>>  # Similar for terminfo
>> -GROUPS["terminfoinkernel"] = [ "emu", "mips_loongson", "mips_arc", 
>> "mips_qemu_mips" ] + GROUPS["xen"] + GROUPS["ieee1275"] + GROUPS["uboot"];
>> +GROUPS["terminfoinkernel"] = [ "emu", "mips_loongson", "mips_arc", 
>> "mips_qemu_mips", "i386_xenpvh" ] + GROUPS["xen"] + GROUPS["ieee1275"] + 
>> GROUPS["uboot"];
>>  GROUPS["terminfomodule"]   = GRUB_PLATFORMS[:];
>>  for i in GROUPS["terminfoinkernel"]: GROUPS["terminfomodule"].remove(i)
>>
>> diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
>> index f4ff62b76..d4417e2c4 100644
>> --- a/grub-core/Makefile.am
>> +++ b/grub-core/Makefile.am
>> @@ -101,6 +101,18 @@ KERNEL_HEADER_FILES += 
>> $(top_builddir)/include/grub/machine/int.h
>>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
>>  endif
>>
>> +if COND_i386_xenpvh
>> +KERNEL_HEADER_FILES += $(top_builddir)/include/grub/machine/kernel.h
>> +KERNEL_HEADER_FILES += $(top_builddir)/include/grub/machine/int.h
>> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
>> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/terminfo.h
>> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/extcmd.h
>> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/loader.h
>> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/lib/arg.h
>> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/xen.h
>> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/xen/hypercall.h
>> +endif
>> +
>>  if COND_i386_efi
>>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/efi.h
>>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/disk.h
>> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
>> index 9590e87d9..c42cebc38 100644
>> --- a/grub-core/Makefile.core.def
>> +++ b/grub-core/Makefile.core.def
>> @@ -79,6 +79,8 @@ kernel = {
>>i386_xen_ldflags = '$(TARGET_IMG_BASE_LDOPT),0';
>>x86_64_xen_ldflags   = '$(TARGET_IMG_LDFLAGS)';
>>x86_64_xen_ldflags   = '$(TARGET_IMG_BASE_LDOPT),0';
>> +  i386_xenpvh_ldflags  = '$(TARGET_IMG_LDFLAGS)';
>> +  i386_xenpvh_ldflags  = '$(TARGET_IMG_BASE_LDOPT),0x10';
>>
>>mips_loongson_ldflags= '-Wl,-Ttext,0x8020';
>>powerpc_ieee1275_ldflags = '-Wl,-Ttext,0x20';
>> @@ -100,6 +102,7 @@ kernel = {
>>x86_64_efi_startup = kern/x86_64/efi/startup.S;
>>i386_xen_startup = kern/i386/xen/startup.S;
>>x86_64_xen_startup = kern/x86_64/xen/startup.S;
>> +  i386_xenpvh_startup = kern/i386/xen/startup_pvh.S;
>>i386_qemu_startup = kern/i386/qemu/startup.S;
>>i386_ieee1275_startup = kern/i386/ieee1275/startup.S;
>>i386_coreboot_startup = kern/i386/coreboot/startup.S;
>> @@ -177,6 +180,7 @@ kernel = {
>>
>>i386 = kern/i386/dl.c;
>>i386_xen = kern/i386/dl.c;
>> +  i386_xenpvh = kern/i386/dl.c;
>>
>>i386_coreboot = kern/i386/coreboot/init.c;
>>i386_multiboot = kern/i386/coreboot/init.c;
>> @@ -222,6 +226,14 @@ kernel = {
>>xen = disk/xen/xendisk.c;
>>xen = commands/boot.c;
>>
>> +  i386_xenpvh = kern/i386/tsc.c;
>> +  i386_xenpvh = kern/i386/xen/tsc.c;
>> +  i386_xenpvh = commands/boot.c;
>> +  i386_xenpvh = kern/xen/init.c;
>> +  i386_xenpvh = kern/i386/xen/pvh.c;
>> +  i386_xenpvh = term/xen/console.c;
>> +  i386_xenpvh = disk/xen/xendisk.c;
> 
> I would sort all file names here.

Okay.


Juergen


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 15/18] grub-module-verifier: Ignore all_video for xenpvh

2018-10-22 Thread Daniel Kiper
On Tue, Oct 09, 2018 at 01:03:14PM +0200, Juergen Gross wrote:
> From: Hans van Kranenburg 
>
> This solves the build failing with "Error: no symbol table and no
> .moddeps section"
>
> Also see:
> - 6371e9c10433578bb236a8284ddb9ce9e201eb59
> - https://savannah.gnu.org/bugs/?49012
>
> Signed-off-by: Hans van Kranenburg 

Except s/xenpvh/xen_pvh/ Reviewed-by: Daniel Kiper 

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 13/18] xen: init memory regions for PVH

2018-10-22 Thread Juergen Gross
On 22/10/2018 13:31, Daniel Kiper wrote:
> On Tue, Oct 09, 2018 at 01:03:12PM +0200, Juergen Gross wrote:
>> Add all usable memory regions to grub memory management and add the
>> needed mmap iterate code.
> 
> I am missing a few words why this patch is needed. Especially why
> grub_machine_mmap_iterate() has to belong to this patch. However,
> I think that it should be introduced by patch in which
> grub_machine_mmap_iterate() is used at some point.

That would again lead to one giant PVH patch which you didn't like.

grub_machine_mmap_iterate() is being used by grub common code like
grub-core/lib/relocator.c or grub-core/mmap/mmap.c

grub_machine_mmap_iterate() belongs into this patch as it is the
main user of the memory map introduced here.


Juergen

> 
>> As we are running in 32-bit mode don't add memory above 4GB.
>>
>> Signed-off-by: Juergen Gross 
>> ---
>>  grub-core/kern/i386/xen/pvh.c | 35 +++
>>  1 file changed, 35 insertions(+)
>>
>> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
>> index 93ed68245..c4a8bccf4 100644
>> --- a/grub-core/kern/i386/xen/pvh.c
>> +++ b/grub-core/kern/i386/xen/pvh.c
>> @@ -222,6 +222,30 @@ grub_xen_get_mmap (void)
>>grub_xen_sort_mmap ();
>>  }
>>
>> +static void
>> +grub_xen_mm_init_regions (void)
>> +{
>> +  grub_uint64_t modend, from, to;
>> +  unsigned int i;
>> +
>> +  modend = grub_modules_get_end ();
>> +
>> +  for (i = 0; i < nr_map_entries; i++)
>> +{
>> +  if (map[i].type != GRUB_MEMORY_AVAILABLE)
>> +continue;
>> +  from = map[i].addr;
>> +  to = from + map[i].len;
>> +  if (from < modend)
>> +from = modend;
>> +  if (from >= to || from >= 0x1ULL)
>> +continue;
>> +  if (to > 0x1ULL)
>> +to = 0x1ULL;
>> +  grub_mm_init_region ((void *) (grub_addr_t) from, to - from);
>> +}
>> +}
>> +
>>  static grub_uint64_t
>>  grub_xen_find_page (grub_uint64_t start)
>>  {
>> @@ -302,10 +326,21 @@ grub_xen_setup_pvh (void)
>>grub_xen_shared_info = grub_xen_add_physmap (XENMAPSPACE_shared_info,
>> (void *) par);
>>
>> +  grub_xen_mm_init_regions ();
>> +
>>grub_rsdp_addr = pvh_start_info->rsdp_paddr;
>>  }
>>
>>  grub_err_t
>>  grub_machine_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
>>  {
>> +  unsigned int i;
>> +
>> +  for (i = 0; i < nr_map_entries; i++)
>> +{
>> +  if (map[i].len && hook (map[i].addr, map[i].len, map[i].type, 
>> hook_data))
>> +break;
>> +}
>> +
>> +  return GRUB_ERR_NONE;
> 
> Daniel
> 


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 14/18] xenpvh: add build runes for grub-core

2018-10-22 Thread Daniel Kiper
On Tue, Oct 09, 2018 at 01:03:13PM +0200, Juergen Gross wrote:
> Add the modifications to the build system needed to build a xenpvh
> grub.
>
> Signed-off-by: Juergen Gross 
> Reviewed-by: Daniel Kiper 
> ---
>  gentpl.py   |  4 ++--
>  grub-core/Makefile.am   | 12 
>  grub-core/Makefile.core.def | 35 +++
>  3 files changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/gentpl.py b/gentpl.py
> index da67965a4..9732b4aee 100644
> --- a/gentpl.py
> +++ b/gentpl.py
> @@ -28,7 +28,7 @@ import re
>
>  GRUB_PLATFORMS = [ "emu", "i386_pc", "i386_efi", "i386_qemu", 
> "i386_coreboot",
> "i386_multiboot", "i386_ieee1275", "x86_64_efi",
> -   "i386_xen", "x86_64_xen",
> +   "i386_xen", "x86_64_xen", "i386_xenpvh",

s/i386_xenpvh/i386_xen_pvh/ here and below please...

> "mips_loongson", "sparc64_ieee1275",
> "powerpc_ieee1275", "mips_arc", "ia64_efi",
> "mips_qemu_mips", "arm_uboot", "arm_efi", "arm64_efi",
> @@ -71,7 +71,7 @@ GROUPS["videomodules"]   = GRUB_PLATFORMS[:];
>  for i in GROUPS["videoinkernel"]: GROUPS["videomodules"].remove(i)
>
>  # Similar for terminfo
> -GROUPS["terminfoinkernel"] = [ "emu", "mips_loongson", "mips_arc", 
> "mips_qemu_mips" ] + GROUPS["xen"] + GROUPS["ieee1275"] + GROUPS["uboot"];
> +GROUPS["terminfoinkernel"] = [ "emu", "mips_loongson", "mips_arc", 
> "mips_qemu_mips", "i386_xenpvh" ] + GROUPS["xen"] + GROUPS["ieee1275"] + 
> GROUPS["uboot"];
>  GROUPS["terminfomodule"]   = GRUB_PLATFORMS[:];
>  for i in GROUPS["terminfoinkernel"]: GROUPS["terminfomodule"].remove(i)
>
> diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
> index f4ff62b76..d4417e2c4 100644
> --- a/grub-core/Makefile.am
> +++ b/grub-core/Makefile.am
> @@ -101,6 +101,18 @@ KERNEL_HEADER_FILES += 
> $(top_builddir)/include/grub/machine/int.h
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
>  endif
>
> +if COND_i386_xenpvh
> +KERNEL_HEADER_FILES += $(top_builddir)/include/grub/machine/kernel.h
> +KERNEL_HEADER_FILES += $(top_builddir)/include/grub/machine/int.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/terminfo.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/extcmd.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/loader.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/lib/arg.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/xen.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/xen/hypercall.h
> +endif
> +
>  if COND_i386_efi
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/efi.h
>  KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/disk.h
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 9590e87d9..c42cebc38 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -79,6 +79,8 @@ kernel = {
>i386_xen_ldflags = '$(TARGET_IMG_BASE_LDOPT),0';
>x86_64_xen_ldflags   = '$(TARGET_IMG_LDFLAGS)';
>x86_64_xen_ldflags   = '$(TARGET_IMG_BASE_LDOPT),0';
> +  i386_xenpvh_ldflags  = '$(TARGET_IMG_LDFLAGS)';
> +  i386_xenpvh_ldflags  = '$(TARGET_IMG_BASE_LDOPT),0x10';
>
>mips_loongson_ldflags= '-Wl,-Ttext,0x8020';
>powerpc_ieee1275_ldflags = '-Wl,-Ttext,0x20';
> @@ -100,6 +102,7 @@ kernel = {
>x86_64_efi_startup = kern/x86_64/efi/startup.S;
>i386_xen_startup = kern/i386/xen/startup.S;
>x86_64_xen_startup = kern/x86_64/xen/startup.S;
> +  i386_xenpvh_startup = kern/i386/xen/startup_pvh.S;
>i386_qemu_startup = kern/i386/qemu/startup.S;
>i386_ieee1275_startup = kern/i386/ieee1275/startup.S;
>i386_coreboot_startup = kern/i386/coreboot/startup.S;
> @@ -177,6 +180,7 @@ kernel = {
>
>i386 = kern/i386/dl.c;
>i386_xen = kern/i386/dl.c;
> +  i386_xenpvh = kern/i386/dl.c;
>
>i386_coreboot = kern/i386/coreboot/init.c;
>i386_multiboot = kern/i386/coreboot/init.c;
> @@ -222,6 +226,14 @@ kernel = {
>xen = disk/xen/xendisk.c;
>xen = commands/boot.c;
>
> +  i386_xenpvh = kern/i386/tsc.c;
> +  i386_xenpvh = kern/i386/xen/tsc.c;
> +  i386_xenpvh = commands/boot.c;
> +  i386_xenpvh = kern/xen/init.c;
> +  i386_xenpvh = kern/i386/xen/pvh.c;
> +  i386_xenpvh = term/xen/console.c;
> +  i386_xenpvh = disk/xen/xendisk.c;

I would sort all file names here.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH v2 13/18] xen: init memory regions for PVH

2018-10-22 Thread Daniel Kiper
On Tue, Oct 09, 2018 at 01:03:12PM +0200, Juergen Gross wrote:
> Add all usable memory regions to grub memory management and add the
> needed mmap iterate code.

I am missing a few words why this patch is needed. Especially why
grub_machine_mmap_iterate() has to belong to this patch. However,
I think that it should be introduced by patch in which
grub_machine_mmap_iterate() is used at some point.

> As we are running in 32-bit mode don't add memory above 4GB.
>
> Signed-off-by: Juergen Gross 
> ---
>  grub-core/kern/i386/xen/pvh.c | 35 +++
>  1 file changed, 35 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index 93ed68245..c4a8bccf4 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -222,6 +222,30 @@ grub_xen_get_mmap (void)
>grub_xen_sort_mmap ();
>  }
>
> +static void
> +grub_xen_mm_init_regions (void)
> +{
> +  grub_uint64_t modend, from, to;
> +  unsigned int i;
> +
> +  modend = grub_modules_get_end ();
> +
> +  for (i = 0; i < nr_map_entries; i++)
> +{
> +  if (map[i].type != GRUB_MEMORY_AVAILABLE)
> +continue;
> +  from = map[i].addr;
> +  to = from + map[i].len;
> +  if (from < modend)
> +from = modend;
> +  if (from >= to || from >= 0x1ULL)
> +continue;
> +  if (to > 0x1ULL)
> +to = 0x1ULL;
> +  grub_mm_init_region ((void *) (grub_addr_t) from, to - from);
> +}
> +}
> +
>  static grub_uint64_t
>  grub_xen_find_page (grub_uint64_t start)
>  {
> @@ -302,10 +326,21 @@ grub_xen_setup_pvh (void)
>grub_xen_shared_info = grub_xen_add_physmap (XENMAPSPACE_shared_info,
>  (void *) par);
>
> +  grub_xen_mm_init_regions ();
> +
>grub_rsdp_addr = pvh_start_info->rsdp_paddr;
>  }
>
>  grub_err_t
>  grub_machine_mmap_iterate (grub_memory_hook_t hook, void *hook_data)
>  {
> +  unsigned int i;
> +
> +  for (i = 0; i < nr_map_entries; i++)
> +{
> +  if (map[i].len && hook (map[i].addr, map[i].len, map[i].type, 
> hook_data))
> +break;
> +}
> +
> +  return GRUB_ERR_NONE;

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [Xen-devel] [PATCH v2 09/18] xen: add PVH boot entry code

2018-10-22 Thread Daniel Kiper
On Fri, Oct 19, 2018 at 04:50:25PM +0200, Juergen Gross wrote:
> On 19/10/2018 14:17, Daniel Kiper wrote:
> > On Tue, Oct 09, 2018 at 01:03:08PM +0200, Juergen Gross wrote:
> >> Add the code for the Xen PVH mode boot entry.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >>  grub-core/kern/i386/xen/startup_pvh.S | 50 
> >> +++
> >>  1 file changed, 50 insertions(+)
> >>
> >> diff --git a/grub-core/kern/i386/xen/startup_pvh.S 
> >> b/grub-core/kern/i386/xen/startup_pvh.S
> >> index e18ee5b31..0ddb63b31 100644
> >> --- a/grub-core/kern/i386/xen/startup_pvh.S
> >> +++ b/grub-core/kern/i386/xen/startup_pvh.S
> >> @@ -19,11 +19,61 @@
> >>
> >>  #include 
> >>  #include 
> >> +#include 
> >>
> >>.file   "startup_pvh.S"
> >>.text
> >> +  .globl  start, _start
> >> +  .code32
> >>
> >> +start:
> >> +_start:
> >> +  cld
> >> +  lgdtgdtdesc
> >> +  ljmp$GRUB_MEMORY_MACHINE_PROT_MODE_CSEG, $1f
> >> +1:
> >> +  movl$GRUB_MEMORY_MACHINE_PROT_MODE_DSEG, %eax
> >> +  mov %eax, %ds
> >> +  mov %eax, %es
> >> +  mov %eax, %ss
> >
> > Should not you load null descriptor into %fs and %gs?
> > Just in case...
>
> Hmm, if you want I can do that, sure.

Please do.

> >> +  lealLOCAL(stack_end), %esp
> >> +
> >> +  /* Save address of start info structure. */
> >> +  mov %ebx, pvh_start_info
> >> +  callEXT_C(grub_main)
> >> +  /* Doesn't return. */
> >> +
> >> +  .p2align3
> >> +gdt:
> >> +  .word   0, 0
> >> +  .byte   0, 0, 0, 0
> >> +
> >> +  /* -- code segment --
> >> +   * base = 0x, limit = 0xF (4 KiB Granularity), present
> >> +   * type = 32bit code execute/read, DPL = 0
> >> +   */
> >> +  .word   0x, 0
> >> +  .byte   0, 0x9A, 0xCF, 0
> >> +
> >> +  /* -- data segment --
> >> +   * base = 0x, limit 0xF (4 KiB Granularity), present
> >> +   * type = 32 bit data read/write, DPL = 0
> >> +   */
> >> +  .word   0x, 0
> >> +  .byte   0, 0x92, 0xCF, 0
> >> +
> >> +  .p2align3
> >> +/* this is the GDT descriptor */
> >> +gdtdesc:
> >> +  .word   0x17/* limit */
> >> +  .long   gdt /* addr */
> >> +
> >> +  .p2align2
> >>  /* Saved pointer to start info structure. */
> >>.globl  pvh_start_info
> >>  pvh_start_info:
> >>.long   0
> >> +
> >> +  .bss
> >> +  .space  (1 << 22)
> >
> > Hmmm... Why do we need 4 MiB here? If this is really needed then it begs for
> > a comment. And I would like to see a constant instead of plain number here.
>
> This is just copied from xen/startup.S
>
> I can reduce it to something near GRUB_MEMORY_MACHINE_PROT_STACK_SIZE
> (about 64kB).

GRUB_MEMORY_MACHINE_PROT_STACK_SIZE works for me.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [Xen-devel] [PATCH v2 08/18] xen: add basic hooks for PVH in current code

2018-10-22 Thread Juergen Gross
On 22/10/2018 13:16, Daniel Kiper wrote:
> On Fri, Oct 19, 2018 at 05:52:44PM +0200, Juergen Gross wrote:
>> On 19/10/2018 17:33, Roger Pau Monné wrote:
>>> On Tue, Oct 09, 2018 at 01:03:07PM +0200, Juergen Gross wrote:
 Add the hooks to current code needed for Xen PVH.

 Signed-off-by: Juergen Gross 
 ---
  grub-core/kern/i386/xen/pvh.c | 36 
 +++
  grub-core/kern/i386/xen/startup_pvh.S | 29 
  grub-core/kern/xen/init.c |  6 ++
  include/grub/i386/xenpvh/kernel.h | 30 +
  include/grub/xen.h|  6 ++
  5 files changed, 107 insertions(+)
  create mode 100644 grub-core/kern/i386/xen/pvh.c
  create mode 100644 grub-core/kern/i386/xen/startup_pvh.S
  create mode 100644 include/grub/i386/xenpvh/kernel.h

 diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
 new file mode 100644
 index 0..182ef95f9
 --- /dev/null
 +++ b/grub-core/kern/i386/xen/pvh.c
 @@ -0,0 +1,36 @@
 +/*
 + *  GRUB  --  GRand Unified Bootloader
 + *  Copyright (C) 2011  Free Software Foundation, Inc.
>>>
>>> Isn't this header (and the ones below) kind of off at least year
>>> wise?
>>
>> Hmm, only a little bit :-)
>>
>> Will update.
> 
> Please do this for all headers in the patchset

That was my plan :-)


Juergen

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [Xen-devel] [PATCH v2 08/18] xen: add basic hooks for PVH in current code

2018-10-22 Thread Daniel Kiper
On Fri, Oct 19, 2018 at 05:52:44PM +0200, Juergen Gross wrote:
> On 19/10/2018 17:33, Roger Pau Monné wrote:
> > On Tue, Oct 09, 2018 at 01:03:07PM +0200, Juergen Gross wrote:
> >> Add the hooks to current code needed for Xen PVH.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >>  grub-core/kern/i386/xen/pvh.c | 36 
> >> +++
> >>  grub-core/kern/i386/xen/startup_pvh.S | 29 
> >>  grub-core/kern/xen/init.c |  6 ++
> >>  include/grub/i386/xenpvh/kernel.h | 30 +
> >>  include/grub/xen.h|  6 ++
> >>  5 files changed, 107 insertions(+)
> >>  create mode 100644 grub-core/kern/i386/xen/pvh.c
> >>  create mode 100644 grub-core/kern/i386/xen/startup_pvh.S
> >>  create mode 100644 include/grub/i386/xenpvh/kernel.h
> >>
> >> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> >> new file mode 100644
> >> index 0..182ef95f9
> >> --- /dev/null
> >> +++ b/grub-core/kern/i386/xen/pvh.c
> >> @@ -0,0 +1,36 @@
> >> +/*
> >> + *  GRUB  --  GRand Unified Bootloader
> >> + *  Copyright (C) 2011  Free Software Foundation, Inc.
> >
> > Isn't this header (and the ones below) kind of off at least year
> > wise?
>
> Hmm, only a little bit :-)
>
> Will update.

Please do this for all headers in the patchset

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH V10] Add support for BTRFS raid5/6 to GRUB

2018-10-22 Thread Daniel Kiper
On Thu, Oct 18, 2018 at 07:55:32PM +0200, Goffredo Baroncelli wrote:
>
> Hi All,
>
> the aim of this patches set is to provide support for a BTRFS raid5/6
> filesystem in GRUB.
>
> The first patch, implements the basic support for raid5/6. I.e this works when
> all the disks are present.
>
> The next 5 patches, are preparatory ones.
>
> The 7th patch implements the raid5 recovery for btrfs (i.e. handling the
> disappearing of 1 disk).
> The 8th patch makes the code for handling the raid6 recovery more generic.
> The last one implements the raid6 recovery for btrfs (i.e. handling the
> disappearing up to two disks).
>
> I tested the code in grub-emu, and it works both with all the disks,
> and with some disks missing. I checked the crc32 calculated from grub and
> from linux and these matched. Finally I checked if the support for md raid6
> still works properly, and it does (with all drives and with up to 2 drives
> missing)
>
> Comments are welcome.

In general whole patch series LGTM. +/- some nit picks including changes
for patch #7. If you are OK with them and there are no objections then
I will apply the patches in a week or so.

Thank you for doing the work.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 7/9] btrfs: Add support for recovery for a RAID 5 btrfs profiles.

2018-10-22 Thread Daniel Kiper
On Thu, Oct 18, 2018 at 07:55:39PM +0200, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli 
>
> Add support for recovery for a RAID 5 btrfs profile. In addition
> it is added some code as preparatory work for RAID 6 recovery code.
>
> Signed-off-by: Goffredo Baroncelli 
> ---
>  grub-core/fs/btrfs.c | 161 +--
>  1 file changed, 156 insertions(+), 5 deletions(-)
>
> diff --git a/grub-core/fs/btrfs.c b/grub-core/fs/btrfs.c
> index ea97f0502..b277f2904 100644
> --- a/grub-core/fs/btrfs.c
> +++ b/grub-core/fs/btrfs.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -665,6 +666,140 @@ btrfs_read_from_chunk (struct grub_btrfs_data *data,
>  return err;
>  }
>
> +struct raid56_buffer {
> +  void *buf;
> +  int  data_is_valid;
> +};
> +
> +static void
> +rebuild_raid5 (char *dest, struct raid56_buffer *buffers,
> +grub_uint64_t nstripes, grub_uint64_t csize)
> +{
> +  grub_uint64_t i;
> +  int first;
> +
> +  for(i = 0; buffers[i].data_is_valid && i < nstripes; i++);
> +
> +  if (i == nstripes)
> +{
> +  grub_dprintf ("btrfs", "called rebuild_raid5(), but all disks are 
> OK\n");
> +  return;
> +}
> +
> +  grub_dprintf ("btrfs", "rebuilding RAID 5 stripe #%" PRIuGRUB_UINT64_T 
> "\n", i);
> +
> +  for (i = 0, first = 1; i < nstripes; i++)
> +{
> +  if (!buffers[i].data_is_valid)
> + continue;
> +
> +  if (first) {
> + grub_memcpy(dest, buffers[i].buf, csize);
> + first = 0;
> +  } else
> + grub_crypto_xor (dest, dest, buffers[i].buf, csize);
> +}
> +}
> +
> +static grub_err_t
> +raid56_read_retry (struct grub_btrfs_data *data,
> +struct grub_btrfs_chunk_item *chunk,
> +grub_uint64_t stripe_offset,
> +grub_uint64_t csize, void *buf)
> +{
> +  struct raid56_buffer *buffers;
> +  grub_uint64_t nstripes = grub_le_to_cpu16 (chunk->nstripes);
> +  grub_uint64_t chunk_type = grub_le_to_cpu64 (chunk->type);
> +  grub_err_t ret = GRUB_ERR_OUT_OF_MEMORY;
> +  grub_uint64_t i, failed_devices;
> +
> +  buffers = grub_zalloc (sizeof(*buffers) * nstripes);
> +  if (!buffers)
> +goto cleanup;
> +
> +  for (i = 0; i < nstripes; i++)
> +{
> +  buffers[i].buf = grub_zalloc (csize);
> +  if (!buffers[i].buf)
> + goto cleanup;
> +}
> +
> +  for (failed_devices = 0, i = 0; i < nstripes; i++)
> +{
> +  struct grub_btrfs_chunk_stripe *stripe;
> +  grub_disk_addr_t paddr;
> +  grub_device_t dev;
> +  grub_err_t err;
> +
> +  /* The struct grub_btrfs_chunk_stripe array lives behind struct
> +  grub_btrfs_chunk_item. */
> +  stripe = (struct grub_btrfs_chunk_stripe *) (chunk + 1) + i;
> +
> +  paddr = grub_le_to_cpu64 (stripe->offset) + stripe_offset;
> +  grub_dprintf ("btrfs", "reading paddr %" PRIxGRUB_UINT64_T
> +" from stripe ID %" PRIxGRUB_UINT64_T "\n", paddr,
> +stripe->device_id);
> +
> +  dev = find_device (data, stripe->device_id);
> +  if (!dev)
> + {
> +   buffers[i].data_is_valid = 0;

You do not need this and...

> +   grub_dprintf ("btrfs", "stripe %" PRIuGRUB_UINT64_T " FAILED (dev ID 
> %"
> + PRIxGRUB_UINT64_T ")\n", i, stripe->device_id);
> +   failed_devices++;
> +   continue;
> + }
> +
> +  err = grub_disk_read (dev->disk, paddr >> GRUB_DISK_SECTOR_BITS,
> + paddr & (GRUB_DISK_SECTOR_SIZE - 1),
> + csize, buffers[i].buf);
> +  if (err == GRUB_ERR_NONE)
> + {
> +   buffers[i].data_is_valid = 1;
> +   grub_dprintf ("btrfs", "stripe %" PRIuGRUB_UINT64_T " OK (dev ID %"
> + PRIxGRUB_UINT64_T ")\n", i, stripe->device_id);
> + }
> +  else
> + {
> +   buffers[i].data_is_valid = 0;

...this. grub_zalloc() above did work for you.

> +   grub_dprintf ("btrfs", "stripe %" PRIuGRUB_UINT64_T
> + " FAILED (dev ID %" PRIxGRUB_UINT64_T ")\n", i,

s/FAILED/READ FAILED/

Otherwise Reviewed-by: Daniel Kiper 

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel