Re: [Xen-devel] [PATCH v2 12/18] xen: setup Xen specific data for PVH
On 19/10/2018 18:10, Roger Pau Monné wrote:
> On Tue, Oct 09, 2018 at 01:03:11PM +0200, Juergen Gross wrote:
>> Initialize the needed Xen specific data. This is:
>>
>> - the Xen start of day page containing the console and Xenstore ring
>> page PFN and event channel
>> - the grant table
>> - the shared info page
>>
>> Set the RSDP address for the guest from the start_info page passed
>> as boot parameter.
>>
>> Signed-off-by: Juergen Gross
>> ---
>> grub-core/kern/i386/xen/pvh.c | 107
>> ++
>> 1 file changed, 107 insertions(+)
>>
>> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
>> index b4933b454..93ed68245 100644
>> --- a/grub-core/kern/i386/xen/pvh.c
>> +++ b/grub-core/kern/i386/xen/pvh.c
>> @@ -24,6 +24,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>
>> struct xen_machine_mmap_entry
>> @@ -39,6 +40,7 @@ static struct { char _entry[32]; } hypercall_page[128]
>>__attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>>
>> static grub_uint32_t xen_cpuid_base;
>> +static struct start_info grub_xen_start_page;
>> static struct xen_machine_mmap_entry map[128];
>> static unsigned int nr_map_entries;
>>
>> @@ -104,6 +106,36 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t
>> a0,
>>return __res;
>> }
>>
>> +static grub_uint32_t
>> +grub_xen_get_param (int idx)
>> +{
>> + struct xen_hvm_param xhv;
>> + int r;
>> +
>> + xhv.domid = DOMID_SELF;
>> + xhv.index = idx;
>> + r = grub_xen_hypercall (__HYPERVISOR_hvm_op, HVMOP_get_param,
>> + (grub_uint32_t) (), 0, 0, 0, 0);
>> + if (r < 0)
>> +grub_xen_early_halt ();
>> + return xhv.value;
>> +}
>> +
>> +static void *
>> +grub_xen_add_physmap (unsigned int space, void *addr)
>> +{
>> + struct xen_add_to_physmap xatp;
>> +
>> + xatp.domid = DOMID_SELF;
>> + xatp.idx = 0;
>> + xatp.space = space;
>> + xatp.gpfn = (grub_addr_t) addr >> GRUB_XEN_LOG_PAGE_SIZE;
>> + if (grub_xen_hypercall (__HYPERVISOR_memory_op, XENMEM_add_to_physmap,
>> + (grub_uint32_t) (), 0, 0, 0, 0))
>> +grub_xen_early_halt ();
>> + return addr;
>> +}
>> +
>> static void
>> grub_xen_sort_mmap (void)
>> {
>> @@ -190,12 +222,87 @@ grub_xen_get_mmap (void)
>>grub_xen_sort_mmap ();
>> }
>>
>> +static grub_uint64_t
>> +grub_xen_find_page (grub_uint64_t start)
>> +{
>> + unsigned int i, j;
>> + grub_uint64_t last = start;
>> +
>> + /* Try to find a e820 map hole below 4G. */
>
> Doing this is kind of dangerous, what if you end up placing something
> on top of an MMIO region (either emulated or from a real passthrough
> device)?
Shouldn't those be marked as "Reserved" in the memory map?
>
>> + for (i = 0; i < nr_map_entries; i++)
>> +{
>> + if (last > map[i].addr + map[i].len)
>> +continue;
>> + if (last < map[i].addr)
>> +return last;
>> + if ((map[i].addr >> 32) || ((map[i].addr + map[i].len) >> 32))
>> +break;
>> + last = map[i].addr + map[i].len;
>> +}
>> +if (i == nr_map_entries)
>> + return last;
>> +
>> + /* No hole found, use the highest RAM page below 4G and reserve it. */
>
> I would rather use a RAM page and populate if afterwards, so that the
> memory map returned by the Xen hypercall still matches the current
> physmap, or else update the memmap using XENMEM_set_memory_map?
>
> This has the nasty side effect of shattering the p2m though.
Right, and this would be a noticeable performance hit.
Juergen
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Xen-devel] [PATCH v2 12/18] xen: setup Xen specific data for PVH
On Tue, Oct 09, 2018 at 01:03:11PM +0200, Juergen Gross wrote:
> Initialize the needed Xen specific data. This is:
>
> - the Xen start of day page containing the console and Xenstore ring
> page PFN and event channel
> - the grant table
> - the shared info page
>
> Set the RSDP address for the guest from the start_info page passed
> as boot parameter.
>
> Signed-off-by: Juergen Gross
> ---
> grub-core/kern/i386/xen/pvh.c | 107
> ++
> 1 file changed, 107 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index b4933b454..93ed68245 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> struct xen_machine_mmap_entry
> @@ -39,6 +40,7 @@ static struct { char _entry[32]; } hypercall_page[128]
>__attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>
> static grub_uint32_t xen_cpuid_base;
> +static struct start_info grub_xen_start_page;
> static struct xen_machine_mmap_entry map[128];
> static unsigned int nr_map_entries;
>
> @@ -104,6 +106,36 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t
> a0,
>return __res;
> }
>
> +static grub_uint32_t
> +grub_xen_get_param (int idx)
> +{
> + struct xen_hvm_param xhv;
> + int r;
> +
> + xhv.domid = DOMID_SELF;
> + xhv.index = idx;
> + r = grub_xen_hypercall (__HYPERVISOR_hvm_op, HVMOP_get_param,
> + (grub_uint32_t) (), 0, 0, 0, 0);
> + if (r < 0)
> +grub_xen_early_halt ();
> + return xhv.value;
> +}
> +
> +static void *
> +grub_xen_add_physmap (unsigned int space, void *addr)
> +{
> + struct xen_add_to_physmap xatp;
> +
> + xatp.domid = DOMID_SELF;
> + xatp.idx = 0;
> + xatp.space = space;
> + xatp.gpfn = (grub_addr_t) addr >> GRUB_XEN_LOG_PAGE_SIZE;
> + if (grub_xen_hypercall (__HYPERVISOR_memory_op, XENMEM_add_to_physmap,
> + (grub_uint32_t) (), 0, 0, 0, 0))
> +grub_xen_early_halt ();
> + return addr;
> +}
> +
> static void
> grub_xen_sort_mmap (void)
> {
> @@ -190,12 +222,87 @@ grub_xen_get_mmap (void)
>grub_xen_sort_mmap ();
> }
>
> +static grub_uint64_t
> +grub_xen_find_page (grub_uint64_t start)
> +{
> + unsigned int i, j;
> + grub_uint64_t last = start;
> +
> + /* Try to find a e820 map hole below 4G. */
Doing this is kind of dangerous, what if you end up placing something
on top of an MMIO region (either emulated or from a real passthrough
device)?
> + for (i = 0; i < nr_map_entries; i++)
> +{
> + if (last > map[i].addr + map[i].len)
> + continue;
> + if (last < map[i].addr)
> + return last;
> + if ((map[i].addr >> 32) || ((map[i].addr + map[i].len) >> 32))
> + break;
> + last = map[i].addr + map[i].len;
> +}
> +if (i == nr_map_entries)
> + return last;
> +
> + /* No hole found, use the highest RAM page below 4G and reserve it. */
I would rather use a RAM page and populate if afterwards, so that the
memory map returned by the Xen hypercall still matches the current
physmap, or else update the memmap using XENMEM_set_memory_map?
This has the nasty side effect of shattering the p2m though.
Thanks, Roger.
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Xen-devel] [PATCH v2 10/18] xen: setup hypercall page for PVH
On 19/10/2018 17:40, Roger Pau Monné wrote:
> On Tue, Oct 09, 2018 at 01:03:09PM +0200, Juergen Gross wrote:
>> Add the needed code to setup the hypercall page for calling into the
>> Xen hypervisor.
>>
>> Signed-off-by: Juergen Gross
>> ---
>> grub-core/kern/i386/xen/pvh.c | 70
>> +++
>> 1 file changed, 70 insertions(+)
>>
>> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
>> index 182ef95f9..c1b1cf8db 100644
>> --- a/grub-core/kern/i386/xen/pvh.c
>> +++ b/grub-core/kern/i386/xen/pvh.c
>> @@ -20,14 +20,84 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>>
>> grub_uint64_t grub_rsdp_addr;
>>
>> +static struct { char _entry[32]; } hypercall_page[128]
>> + __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>> +
>> +static grub_uint32_t xen_cpuid_base;
>> +
>> +static void
>> +grub_xen_early_halt (void)
>
> I would rename this to grub_xen_early_crash
>
>> +{
>
> And you can use the 0xe9 IO port to print a crash debug message even
> before initializing the hypercall page, I think this would be helpful
> here.
Aah, I didn't know that.
Thanks, I'll change grub_xen_early_halt() to grub_xen_panic() taking a
string as parameter.
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
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. Juergen ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Xen-devel] [PATCH v2 12/18] xen: setup Xen specific data for PVH
On Tue, Oct 09, 2018 at 01:03:11PM +0200, Juergen Gross wrote:
> Initialize the needed Xen specific data. This is:
>
> - the Xen start of day page containing the console and Xenstore ring
> page PFN and event channel
> - the grant table
> - the shared info page
>
> Set the RSDP address for the guest from the start_info page passed
> as boot parameter.
>
> Signed-off-by: Juergen Gross
> ---
> grub-core/kern/i386/xen/pvh.c | 107
> ++
> 1 file changed, 107 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index b4933b454..93ed68245 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> struct xen_machine_mmap_entry
> @@ -39,6 +40,7 @@ static struct { char _entry[32]; } hypercall_page[128]
>__attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>
> static grub_uint32_t xen_cpuid_base;
> +static struct start_info grub_xen_start_page;
> static struct xen_machine_mmap_entry map[128];
> static unsigned int nr_map_entries;
>
> @@ -104,6 +106,36 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t
> a0,
>return __res;
> }
>
> +static grub_uint32_t
> +grub_xen_get_param (int idx)
> +{
> + struct xen_hvm_param xhv;
> + int r;
> +
> + xhv.domid = DOMID_SELF;
> + xhv.index = idx;
> + r = grub_xen_hypercall (__HYPERVISOR_hvm_op, HVMOP_get_param,
> + (grub_uint32_t) (), 0, 0, 0, 0);
> + if (r < 0)
> +grub_xen_early_halt ();
At this point you already have access to the hypervisor console from
the hypercall page, or alternatively you can use the 0xe9 IO port, IMO
a debug message should be printed instead of just halting. Or else
debugging failures is going to be quite complicated.
I would suggest to convert grub_xen_early_halt into
grub_xen_early_crash and make it a printf like function, or at least
take a string.
Thanks, Roger.
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Xen-devel] [PATCH v2 10/18] xen: setup hypercall page for PVH
On Tue, Oct 09, 2018 at 01:03:09PM +0200, Juergen Gross wrote:
> Add the needed code to setup the hypercall page for calling into the
> Xen hypervisor.
>
> Signed-off-by: Juergen Gross
> ---
> grub-core/kern/i386/xen/pvh.c | 70
> +++
> 1 file changed, 70 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index 182ef95f9..c1b1cf8db 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -20,14 +20,84 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> grub_uint64_t grub_rsdp_addr;
>
> +static struct { char _entry[32]; } hypercall_page[128]
> + __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
> +
> +static grub_uint32_t xen_cpuid_base;
> +
> +static void
> +grub_xen_early_halt (void)
I would rename this to grub_xen_early_crash
> +{
And you can use the 0xe9 IO port to print a crash debug message even
before initializing the hypercall page, I think this would be helpful
here.
Roger.
___
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
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? Roger. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Xen-devel] [PATCH v2 12/18] xen: setup Xen specific data for PVH
On 19/10/2018 14:48, Daniel Kiper wrote:
> On Tue, Oct 09, 2018 at 01:03:11PM +0200, Juergen Gross wrote:
>> Initialize the needed Xen specific data. This is:
>>
>> - the Xen start of day page containing the console and Xenstore ring
>> page PFN and event channel
>> - the grant table
>> - the shared info page
>>
>> Set the RSDP address for the guest from the start_info page passed
>> as boot parameter.
>>
>> Signed-off-by: Juergen Gross
>> ---
>> grub-core/kern/i386/xen/pvh.c | 107
>> ++
>> 1 file changed, 107 insertions(+)
>>
>> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
>> index b4933b454..93ed68245 100644
>> --- a/grub-core/kern/i386/xen/pvh.c
>> +++ b/grub-core/kern/i386/xen/pvh.c
>> @@ -24,6 +24,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>
>> struct xen_machine_mmap_entry
>> @@ -39,6 +40,7 @@ static struct { char _entry[32]; } hypercall_page[128]
>>__attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>>
>> static grub_uint32_t xen_cpuid_base;
>> +static struct start_info grub_xen_start_page;
>> static struct xen_machine_mmap_entry map[128];
>> static unsigned int nr_map_entries;
>>
>> @@ -104,6 +106,36 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t
>> a0,
>>return __res;
>> }
>>
>> +static grub_uint32_t
>> +grub_xen_get_param (int idx)
>> +{
>> + struct xen_hvm_param xhv;
>> + int r;
>> +
>> + xhv.domid = DOMID_SELF;
>> + xhv.index = idx;
>> + r = grub_xen_hypercall (__HYPERVISOR_hvm_op, HVMOP_get_param,
>> + (grub_uint32_t) (), 0, 0, 0, 0);
>
> s/(grub_uint32_t) ()/(grub_uint32_t)()/
> Here and in the other patches...
Oh, I have seen lots of places where casts are followed by a blank. I
thought this would be the preferred style.
>
>> + if (r < 0)
>> +grub_xen_early_halt ();
>> + return xhv.value;
>> +}
>> +
>> +static void *
>> +grub_xen_add_physmap (unsigned int space, void *addr)
>> +{
>> + struct xen_add_to_physmap xatp;
>> +
>> + xatp.domid = DOMID_SELF;
>> + xatp.idx = 0;
>> + xatp.space = space;
>> + xatp.gpfn = (grub_addr_t) addr >> GRUB_XEN_LOG_PAGE_SIZE;
>> + if (grub_xen_hypercall (__HYPERVISOR_memory_op, XENMEM_add_to_physmap,
>> + (grub_uint32_t) (), 0, 0, 0, 0))
>> +grub_xen_early_halt ();
>> + return addr;
>> +}
>> +
>> static void
>> grub_xen_sort_mmap (void)
>> {
>> @@ -190,12 +222,87 @@ grub_xen_get_mmap (void)
>>grub_xen_sort_mmap ();
>> }
>>
>> +static grub_uint64_t
>> +grub_xen_find_page (grub_uint64_t start)
>> +{
>> + unsigned int i, j;
>> + grub_uint64_t last = start;
>> +
>> + /* Try to find a e820 map hole below 4G. */
>> + for (i = 0; i < nr_map_entries; i++)
>> +{
>> + if (last > map[i].addr + map[i].len)
>> +continue;
>> + if (last < map[i].addr)
>> +return last;
>> + if ((map[i].addr >> 32) || ((map[i].addr + map[i].len) >> 32))
>> +break;
>> + last = map[i].addr + map[i].len;
>> +}
>> +if (i == nr_map_entries)
>> + return last;
>> +
>> + /* No hole found, use the highest RAM page below 4G and reserve it. */
>
> It seems to me that this comment should be put before next for().
Hmm, that's a matter of taste, I think. The comment is at the point
where the "no hole found" case is started to be handled. I can move
it down, of course.
>
>> + if (nr_map_entries == ARRAY_SIZE(map))
>> +grub_xen_early_halt ();
>> + j = 0;
>> + for (i = 0; i < nr_map_entries; i++)
>
> for (i = 0, j = 0; i < nr_map_entries; i++)
Okay.
Juergen
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Xen-devel] [PATCH v2 11/18] xen: get memory map from hypervisor for PVH
On 19/10/2018 14:40, Daniel Kiper wrote:
> On Tue, Oct 09, 2018 at 01:03:10PM +0200, Juergen Gross wrote:
>> Retrieve the memory map from the hypervisor and normalize it to contain
>> no overlapping entries and to be sorted by address.
>>
>> Signed-off-by: Juergen Gross
>> ---
>> grub-core/kern/i386/xen/pvh.c | 98
>> +++
>> 1 file changed, 98 insertions(+)
>>
>> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
>> index c1b1cf8db..b4933b454 100644
>> --- a/grub-core/kern/i386/xen/pvh.c
>> +++ b/grub-core/kern/i386/xen/pvh.c
>> @@ -22,7 +22,16 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> +#include
>> +
>> +struct xen_machine_mmap_entry
>> +{
>> + grub_uint64_t addr;
>> + grub_uint64_t len;
>> + grub_uint32_t type;
>> +} GRUB_PACKED;
>
> Could not we reuse grub_e820_mmap_entry here?
This is defined only locally in grub-core/mmap/i386/pc/mmap.c
I can move it into an appropriate header and use it here and in mmap.c
>
>> grub_uint64_t grub_rsdp_addr;
>>
>> @@ -30,6 +39,8 @@ static struct { char _entry[32]; } hypercall_page[128]
>>__attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>>
>> static grub_uint32_t xen_cpuid_base;
>> +static struct xen_machine_mmap_entry map[128];
>
> A constant instead of 128? If no why 128?
I can use a macro instead.
>
>> +static unsigned int nr_map_entries;
>>
>> static void
>> grub_xen_early_halt (void)
>> @@ -93,11 +104,98 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t
>> a0,
>>return __res;
>> }
>>
>> +static void
>> +grub_xen_sort_mmap (void)
>> +{
>> + grub_uint64_t from, to;
>> + unsigned int i;
>> + struct xen_machine_mmap_entry tmp;
>> +
>> + /* Align map entries to page boundaries. */
>> + for (i = 0; i < nr_map_entries; i++)
>> +{
>> + from = map[i].addr;
>> + to = from + map[i].len;
>> + if (map[i].type == GRUB_MEMORY_AVAILABLE)
>> +{
>> + from = ALIGN_UP(from, GRUB_XEN_PAGE_SIZE);
>
> Lack of space between macro name and "(".
> Here and below...
Okay.
>
>> + to = ALIGN_DOWN(to, GRUB_XEN_PAGE_SIZE);
>> +}
>> + else
>> +{
>> + from = ALIGN_DOWN(from, GRUB_XEN_PAGE_SIZE);
>> + to = ALIGN_UP(to, GRUB_XEN_PAGE_SIZE);
>> +}
>> + map[i].addr = from;
>> + map[i].len = to - from;
>> +}
Juergen
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Xen-devel] [PATCH v2 10/18] xen: setup hypercall page for PVH
On 19/10/2018 14:30, Daniel Kiper wrote:
> On Tue, Oct 09, 2018 at 01:03:09PM +0200, Juergen Gross wrote:
>> Add the needed code to setup the hypercall page for calling into the
>> Xen hypervisor.
>>
>> Signed-off-by: Juergen Gross
>> ---
>> grub-core/kern/i386/xen/pvh.c | 70
>> +++
>> 1 file changed, 70 insertions(+)
>>
>> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
>> index 182ef95f9..c1b1cf8db 100644
>> --- a/grub-core/kern/i386/xen/pvh.c
>> +++ b/grub-core/kern/i386/xen/pvh.c
>> @@ -20,14 +20,84 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>>
>> grub_uint64_t grub_rsdp_addr;
>>
>> +static struct { char _entry[32]; } hypercall_page[128]
>> + __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>> +
>> +static grub_uint32_t xen_cpuid_base;
>> +
>> +static void
>> +grub_xen_early_halt (void)
>> +{
>> + asm volatile ("hlt");
>> +}
>> +
>> +static void
>> +grub_xen_cpuid_base (void)
>> +{
>> + grub_uint32_t base, eax, signature[3];
>> +
>> + for (base = 0x4000; base < 0x4001; base += 0x100)
>> +{
>> + grub_cpuid (base, eax, signature[0], signature[1], signature[2]);
>> + if (!grub_memcmp ("XenVMMXenVMM", signature, 12) && (eax - base) >= 2)
>> +{
>> + xen_cpuid_base = base;
>> + return;
>> +}
>> +}
>> +
>> + grub_xen_early_halt ();
>> +}
>> +
>> +static void
>> +grub_xen_setup_hypercall_page (void)
>> +{
>> + grub_uint32_t msr, pfn, eax, ebx, ecx, edx;
>> +
>> + grub_cpuid (xen_cpuid_base + 2, eax, ebx, ecx, edx);
>> + msr = ebx;
>> + pfn = (grub_uint32_t) (_page[0]);
>> +
>> + asm volatile ("wrmsr" : : "c" (msr), "a" (pfn), "d" (0) : "memory");
>> +}
>> +
>> +int
>> +grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t a0,
>> +grub_uint32_t a1, grub_uint32_t a2,
>> +grub_uint32_t a3, grub_uint32_t a4,
>> +grub_uint32_t a5 __attribute__ ((unused)))
>> +{
>> + register unsigned long __res asm("eax");
>> + register unsigned long __arg0 asm("ebx") = __arg0;
>> + register unsigned long __arg1 asm("ecx") = __arg1;
>> + register unsigned long __arg2 asm("edx") = __arg2;
>> + register unsigned long __arg3 asm("esi") = __arg3;
>> + register unsigned long __arg4 asm("edi") = __arg4;
>
> Why do we need this play with registers. Does not asm below
> work with "a", "b", ... like above?
Probably, yes. This was inspired by Linux 32-bit version.
I can use the mini-os variant which combines Andrew's suggestion
and yours.
Juergen
___
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
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. > >> +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). 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
On 19/10/2018 14:05, Daniel Kiper wrote: > On Tue, Oct 09, 2018 at 01:03:07PM +0200, Juergen Gross wrote: >> Add the hooks to current code needed for Xen PVH. > > I am not against the code itself but it would be nice to know why you > add, AIUI, just stubs here which will be filled with proper code later. You requested a split of the patch adding PVH stuff. This is the result of that request. Juergen ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Xen-devel] [PATCH v2 07/18] xen: add PVH specific defines to offset.h
On 19/10/2018 13:54, Daniel Kiper wrote: > On Tue, Oct 09, 2018 at 01:03:06PM +0200, Juergen Gross wrote: >> include/grub/offsets.h needs some defines for Xen PVH mode. >> >> Add them. While at it line up the values in the surrounding lines to >> start at the same column. >> >> Signed-off-by: Juergen Gross >> --- >> include/grub/offsets.h | 21 - >> 1 file changed, 12 insertions(+), 9 deletions(-) >> >> diff --git a/include/grub/offsets.h b/include/grub/offsets.h >> index 330e4c707..b16353163 100644 >> --- a/include/grub/offsets.h >> +++ b/include/grub/offsets.h >> @@ -36,9 +36,10 @@ >> #define GRUB_DECOMPRESSOR_I386_PC_MAX_DECOMPRESSOR_SIZE (0x9000-0x8200) >> >> /* The segment where the kernel is loaded. */ >> -#define GRUB_BOOT_I386_PC_KERNEL_SEG0x800 >> +#define GRUB_BOOT_I386_PC_KERNEL_SEG0x800 >> >> -#define GRUB_KERNEL_I386_PC_LINK_ADDR 0x9000 >> +#define GRUB_KERNEL_I386_PC_LINK_ADDR 0x9000 >> +#define GRUB_KERNEL_I386_XENPVH_LINK_ADDR 0x10 > > s/XENPVH/XEN_PVH/ In general I prefer XEN_PVH instead of XENPVH. > So, please update them all where possible. Not only in this patch. Does this apply to path names as well (e.g. include/grub/i386/xenpvh/) ? Or do you mean macros/symbols only? BTW: if yes, this would affect the visible platform name, too. Juergen ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 12/18] xen: setup Xen specific data for PVH
On Tue, Oct 09, 2018 at 01:03:11PM +0200, Juergen Gross wrote:
> Initialize the needed Xen specific data. This is:
>
> - the Xen start of day page containing the console and Xenstore ring
> page PFN and event channel
> - the grant table
> - the shared info page
>
> Set the RSDP address for the guest from the start_info page passed
> as boot parameter.
>
> Signed-off-by: Juergen Gross
> ---
> grub-core/kern/i386/xen/pvh.c | 107
> ++
> 1 file changed, 107 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index b4933b454..93ed68245 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> struct xen_machine_mmap_entry
> @@ -39,6 +40,7 @@ static struct { char _entry[32]; } hypercall_page[128]
>__attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>
> static grub_uint32_t xen_cpuid_base;
> +static struct start_info grub_xen_start_page;
> static struct xen_machine_mmap_entry map[128];
> static unsigned int nr_map_entries;
>
> @@ -104,6 +106,36 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t
> a0,
>return __res;
> }
>
> +static grub_uint32_t
> +grub_xen_get_param (int idx)
> +{
> + struct xen_hvm_param xhv;
> + int r;
> +
> + xhv.domid = DOMID_SELF;
> + xhv.index = idx;
> + r = grub_xen_hypercall (__HYPERVISOR_hvm_op, HVMOP_get_param,
> + (grub_uint32_t) (), 0, 0, 0, 0);
s/(grub_uint32_t) ()/(grub_uint32_t)()/
Here and in the other patches...
> + if (r < 0)
> +grub_xen_early_halt ();
> + return xhv.value;
> +}
> +
> +static void *
> +grub_xen_add_physmap (unsigned int space, void *addr)
> +{
> + struct xen_add_to_physmap xatp;
> +
> + xatp.domid = DOMID_SELF;
> + xatp.idx = 0;
> + xatp.space = space;
> + xatp.gpfn = (grub_addr_t) addr >> GRUB_XEN_LOG_PAGE_SIZE;
> + if (grub_xen_hypercall (__HYPERVISOR_memory_op, XENMEM_add_to_physmap,
> + (grub_uint32_t) (), 0, 0, 0, 0))
> +grub_xen_early_halt ();
> + return addr;
> +}
> +
> static void
> grub_xen_sort_mmap (void)
> {
> @@ -190,12 +222,87 @@ grub_xen_get_mmap (void)
>grub_xen_sort_mmap ();
> }
>
> +static grub_uint64_t
> +grub_xen_find_page (grub_uint64_t start)
> +{
> + unsigned int i, j;
> + grub_uint64_t last = start;
> +
> + /* Try to find a e820 map hole below 4G. */
> + for (i = 0; i < nr_map_entries; i++)
> +{
> + if (last > map[i].addr + map[i].len)
> + continue;
> + if (last < map[i].addr)
> + return last;
> + if ((map[i].addr >> 32) || ((map[i].addr + map[i].len) >> 32))
> + break;
> + last = map[i].addr + map[i].len;
> +}
> +if (i == nr_map_entries)
> + return last;
> +
> + /* No hole found, use the highest RAM page below 4G and reserve it. */
It seems to me that this comment should be put before next for().
> + if (nr_map_entries == ARRAY_SIZE(map))
> +grub_xen_early_halt ();
> + j = 0;
> + for (i = 0; i < nr_map_entries; i++)
for (i = 0, j = 0; i < nr_map_entries; i++)
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Xen-devel] [PATCH v2 10/18] xen: setup hypercall page for PVH
On 19/10/18 13:30, Daniel Kiper wrote:
> On Tue, Oct 09, 2018 at 01:03:09PM +0200, Juergen Gross wrote:
>> +
>> + __arg0 = a0;
>> + __arg1 = a1;
>> + __arg2 = a2;
>> + __arg3 = a3;
>> + __arg4 = a4;
>> + asm volatile ("call *%[callno]"
>> +: "=r" (__res), "+r" (__arg0), "+r" (__arg1), "+r" (__arg2),
>> + "+r" (__arg3), "+r" (__arg4)
>> +: [callno] "a" (_page[callno])
>> +: "memory");
call hypercall_page + %c[offset]
passing [offset] "i" (callno * 32)
which gives you a direct call, rather than an indirect one.
~Andrew
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 10/18] xen: setup hypercall page for PVH
On Tue, Oct 09, 2018 at 01:03:09PM +0200, Juergen Gross wrote:
> Add the needed code to setup the hypercall page for calling into the
> Xen hypervisor.
>
> Signed-off-by: Juergen Gross
> ---
> grub-core/kern/i386/xen/pvh.c | 70
> +++
> 1 file changed, 70 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index 182ef95f9..c1b1cf8db 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -20,14 +20,84 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> grub_uint64_t grub_rsdp_addr;
>
> +static struct { char _entry[32]; } hypercall_page[128]
> + __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
> +
> +static grub_uint32_t xen_cpuid_base;
> +
> +static void
> +grub_xen_early_halt (void)
> +{
> + asm volatile ("hlt");
> +}
> +
> +static void
> +grub_xen_cpuid_base (void)
> +{
> + grub_uint32_t base, eax, signature[3];
> +
> + for (base = 0x4000; base < 0x4001; base += 0x100)
> +{
> + grub_cpuid (base, eax, signature[0], signature[1], signature[2]);
> + if (!grub_memcmp ("XenVMMXenVMM", signature, 12) && (eax - base) >= 2)
> + {
> + xen_cpuid_base = base;
> + return;
> + }
> +}
> +
> + grub_xen_early_halt ();
> +}
> +
> +static void
> +grub_xen_setup_hypercall_page (void)
> +{
> + grub_uint32_t msr, pfn, eax, ebx, ecx, edx;
> +
> + grub_cpuid (xen_cpuid_base + 2, eax, ebx, ecx, edx);
> + msr = ebx;
> + pfn = (grub_uint32_t) (_page[0]);
> +
> + asm volatile ("wrmsr" : : "c" (msr), "a" (pfn), "d" (0) : "memory");
> +}
> +
> +int
> +grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t a0,
> + grub_uint32_t a1, grub_uint32_t a2,
> + grub_uint32_t a3, grub_uint32_t a4,
> + grub_uint32_t a5 __attribute__ ((unused)))
> +{
> + register unsigned long __res asm("eax");
> + register unsigned long __arg0 asm("ebx") = __arg0;
> + register unsigned long __arg1 asm("ecx") = __arg1;
> + register unsigned long __arg2 asm("edx") = __arg2;
> + register unsigned long __arg3 asm("esi") = __arg3;
> + register unsigned long __arg4 asm("edi") = __arg4;
Why do we need this play with registers. Does not asm below
work with "a", "b", ... like above?
> +
> + __arg0 = a0;
> + __arg1 = a1;
> + __arg2 = a2;
> + __arg3 = a3;
> + __arg4 = a4;
> + asm volatile ("call *%[callno]"
> + : "=r" (__res), "+r" (__arg0), "+r" (__arg1), "+r" (__arg2),
> + "+r" (__arg3), "+r" (__arg4)
> + : [callno] "a" (_page[callno])
> + : "memory");
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 11/18] xen: get memory map from hypervisor for PVH
On Tue, Oct 09, 2018 at 01:03:10PM +0200, Juergen Gross wrote:
> Retrieve the memory map from the hypervisor and normalize it to contain
> no overlapping entries and to be sorted by address.
>
> Signed-off-by: Juergen Gross
> ---
> grub-core/kern/i386/xen/pvh.c | 98
> +++
> 1 file changed, 98 insertions(+)
>
> diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c
> index c1b1cf8db..b4933b454 100644
> --- a/grub-core/kern/i386/xen/pvh.c
> +++ b/grub-core/kern/i386/xen/pvh.c
> @@ -22,7 +22,16 @@
> #include
> #include
> #include
> +#include
> #include
> +#include
> +
> +struct xen_machine_mmap_entry
> +{
> + grub_uint64_t addr;
> + grub_uint64_t len;
> + grub_uint32_t type;
> +} GRUB_PACKED;
Could not we reuse grub_e820_mmap_entry here?
> grub_uint64_t grub_rsdp_addr;
>
> @@ -30,6 +39,8 @@ static struct { char _entry[32]; } hypercall_page[128]
>__attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
>
> static grub_uint32_t xen_cpuid_base;
> +static struct xen_machine_mmap_entry map[128];
A constant instead of 128? If no why 128?
> +static unsigned int nr_map_entries;
>
> static void
> grub_xen_early_halt (void)
> @@ -93,11 +104,98 @@ grub_xen_hypercall (grub_uint32_t callno, grub_uint32_t
> a0,
>return __res;
> }
>
> +static void
> +grub_xen_sort_mmap (void)
> +{
> + grub_uint64_t from, to;
> + unsigned int i;
> + struct xen_machine_mmap_entry tmp;
> +
> + /* Align map entries to page boundaries. */
> + for (i = 0; i < nr_map_entries; i++)
> +{
> + from = map[i].addr;
> + to = from + map[i].len;
> + if (map[i].type == GRUB_MEMORY_AVAILABLE)
> + {
> + from = ALIGN_UP(from, GRUB_XEN_PAGE_SIZE);
Lack of space between macro name and "(".
Here and below...
> + to = ALIGN_DOWN(to, GRUB_XEN_PAGE_SIZE);
> + }
> + else
> + {
> + from = ALIGN_DOWN(from, GRUB_XEN_PAGE_SIZE);
> + to = ALIGN_UP(to, GRUB_XEN_PAGE_SIZE);
> + }
> + map[i].addr = from;
> + map[i].len = to - from;
> +}
[...]
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 09/18] xen: add PVH boot entry code
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... > + 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. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 08/18] xen: add basic hooks for PVH in current code
On Tue, Oct 09, 2018 at 01:03:07PM +0200, Juergen Gross wrote: > Add the hooks to current code needed for Xen PVH. I am not against the code itself but it would be nice to know why you add, AIUI, just stubs here which will be filled with proper code later. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 07/18] xen: add PVH specific defines to offset.h
On Tue, Oct 09, 2018 at 01:03:06PM +0200, Juergen Gross wrote: > include/grub/offsets.h needs some defines for Xen PVH mode. > > Add them. While at it line up the values in the surrounding lines to > start at the same column. > > Signed-off-by: Juergen Gross > --- > include/grub/offsets.h | 21 - > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/include/grub/offsets.h b/include/grub/offsets.h > index 330e4c707..b16353163 100644 > --- a/include/grub/offsets.h > +++ b/include/grub/offsets.h > @@ -36,9 +36,10 @@ > #define GRUB_DECOMPRESSOR_I386_PC_MAX_DECOMPRESSOR_SIZE (0x9000-0x8200) > > /* The segment where the kernel is loaded. */ > -#define GRUB_BOOT_I386_PC_KERNEL_SEG 0x800 > +#define GRUB_BOOT_I386_PC_KERNEL_SEG 0x800 > > -#define GRUB_KERNEL_I386_PC_LINK_ADDR 0x9000 > +#define GRUB_KERNEL_I386_PC_LINK_ADDR0x9000 > +#define GRUB_KERNEL_I386_XENPVH_LINK_ADDR0x10 s/XENPVH/XEN_PVH/ In general I prefer XEN_PVH instead of XENPVH. So, please update them all where possible. Not only in this patch. > /* The upper memory area (starting at 640 kiB). */ > #define GRUB_MEMORY_I386_PC_UPPER0xa > @@ -101,15 +102,17 @@ > #define GRUB_KERNEL_I386_MULTIBOOT_MOD_ALIGN > GRUB_KERNEL_I386_COREBOOT_MOD_ALIGN > > #define GRUB_KERNEL_X86_64_XEN_MOD_ALIGN 0x8 > -#define GRUB_KERNEL_I386_XEN_MOD_ALIGN 0x8 > +#define GRUB_KERNEL_I386_XEN_MOD_ALIGN 0x8 > +#define GRUB_KERNEL_I386_XENPVH_MOD_ALIGN0x8 Ditto. > /* Non-zero value is only needed for PowerMacs. */ > -#define GRUB_KERNEL_X86_64_XEN_MOD_GAP 0x0 > -#define GRUB_KERNEL_I386_XEN_MOD_GAP 0x0 > -#define GRUB_KERNEL_I386_IEEE1275_MOD_GAP 0x0 > -#define GRUB_KERNEL_I386_COREBOOT_MOD_GAP 0x0 > -#define GRUB_KERNEL_SPARC64_IEEE1275_MOD_GAP 0x0 > -#define GRUB_KERNEL_ARM_UBOOT_MOD_GAP 0x0 > +#define GRUB_KERNEL_X86_64_XEN_MOD_GAP 0x0 > +#define GRUB_KERNEL_I386_XEN_MOD_GAP 0x0 > +#define GRUB_KERNEL_I386_XENPVH_MOD_GAP 0x0 Ditto. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Xen-devel] [PATCH v2 04/18] xen: prepare common code for Xen PVH support
On 18/10/2018 17:39, Juergen Gross wrote: > On 18/10/2018 16:59, Daniel Kiper wrote: >> On Tue, Oct 09, 2018 at 01:03:03PM +0200, Juergen Gross wrote: >>> Some common code needs to be special cased for Xen PVH mode. This hits >>> mostly Xen PV mode specific areas. >>> >>> Signed-off-by: Juergen Gross >>> --- >>> grub-core/kern/i386/tsc.c | 2 +- >>> include/grub/i386/pc/int.h| 3 +++ >>> include/grub/i386/tsc.h | 2 +- >>> include/grub/i386/xen/hypercall.h | 5 - >>> include/grub/kernel.h | 4 +++- >>> 5 files changed, 12 insertions(+), 4 deletions(-) >>> >>> diff --git a/grub-core/kern/i386/tsc.c b/grub-core/kern/i386/tsc.c >>> index f266eb131..43fee3a13 100644 >>> --- a/grub-core/kern/i386/tsc.c >>> +++ b/grub-core/kern/i386/tsc.c >>> @@ -65,7 +65,7 @@ grub_tsc_init (void) >>> >>>tsc_boot_time = grub_get_tsc (); >>> >>> -#ifdef GRUB_MACHINE_XEN >>> +#if defined (GRUB_MACHINE_XEN) || defined (GRUB_MACHINE_XENPVH) >> >> s/GRUB_MACHINE_XENPVH/GRUB_MACHINE_XEN_PVH/ > > Okay. > >> >>>(void) (grub_tsc_calibrate_from_xen () || calibrate_tsc_hardcode()); >>> #elif defined (GRUB_MACHINE_EFI) >>>(void) (grub_tsc_calibrate_from_pmtimer () || >>> grub_tsc_calibrate_from_pit () || grub_tsc_calibrate_from_efi() || >>> calibrate_tsc_hardcode()); >>> diff --git a/include/grub/i386/pc/int.h b/include/grub/i386/pc/int.h >>> index 16a53e4fe..46fb1e397 100644 >>> --- a/include/grub/i386/pc/int.h >>> +++ b/include/grub/i386/pc/int.h >>> @@ -51,9 +51,12 @@ struct grub_bios_int_registers >>> #define GRUB_CPU_INT_FLAGS_DEFAULT 0 >>> #endif >>> >>> +#ifndef GRUB_MACHINE_XENPVH >>> void EXPORT_FUNC (grub_bios_interrupt) (grub_uint8_t intno, >>> struct grub_bios_int_registers *regs) >>> __attribute__ ((regparm(3))); >>> +#endif >> >> Is it an issue with this declaration? I think that you should take care >> about grub-core/kern/i386/int.S. So, relevant Makefile should be >> updated instead of this declaration. > > I'll have a try. I just remember I struggled a lot with this issue > when writing the patches. Ah, now I've found the problem: I'm including grub/i386/pc/int.h from grub/i386/xenpvh/int.h in order to avoid having to redefine all the macros and structs in that file. Unfortunately the EXPORT_FUNC() will make the build fail as in Xen-PVH I don't have the symbol grub_bios_interrupt available. I see the following solutions: 1. keep the patch as it is now 2. duplicate grub/i386/pc/int.h in grub/i386/xenpvh/int.h without EXPORT_FUNC (grub_bios_interrupt) 3. add a dummy grub_bios_interrupt entry in Xen-PVH code 4. split grub/i386/pc/int.h into int_types.h and int.h with int_types.h containing the stuff I need for Xen-PVH and include int_types.h from grub/i386/pc/int.h and grub/i386/xenpvh/int.h (grub/i386/pc/int.h would contain the EXPORT_FUNC then) What is your preference? Juergen ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
