[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 1/2] x86: Parse Multiboot2 framebuffer information
On 09.02.2022 14:09, Tu Dinh Ngoc wrote: > --- a/xen/arch/x86/boot/reloc.c > +++ b/xen/arch/x86/boot/reloc.c > @@ -156,6 +156,8 @@ static multiboot_info_t *mbi2_reloc(u32 mbi_in) > multiboot_info_t *mbi_out; > u32 ptr; > unsigned int i, mod_idx = 0; > + u64 fbaddr; > + u8 fbtype; Style: Despite adjacent pre-existing code doing so, new code should not be using u<N> anymore, but use uint<N>_t instead. > @@ -254,6 +256,26 @@ static multiboot_info_t *mbi2_reloc(u32 mbi_in) > ++mod_idx; > break; > > + case MULTIBOOT2_TAG_TYPE_FRAMEBUFFER: > + fbaddr = get_mb2_data(tag, framebuffer, framebuffer_addr); > + fbtype = get_mb2_data(tag, framebuffer, framebuffer_type); > + if (fbaddr == 0 || fbtype != MULTIBOOT2_FRAMEBUFFER_TYPE_RGB) Style: Blanks needed immediately inside the parentheses. > + break; > + mbi_out->flags |= MBI_FB; > + mbi_out->fb.addr = fbaddr; > + mbi_out->fb.pitch = get_mb2_data(tag, framebuffer, > framebuffer_pitch); > + mbi_out->fb.width = get_mb2_data(tag, framebuffer, > framebuffer_width); > + mbi_out->fb.height = get_mb2_data(tag, framebuffer, > framebuffer_height); > + mbi_out->fb.bpp = get_mb2_data(tag, framebuffer, > framebuffer_bpp); > + mbi_out->fb.type = fbtype; > + mbi_out->fb.red_pos = get_mb2_data(tag, framebuffer, > framebuffer_red_field_position); > + mbi_out->fb.red_size = get_mb2_data(tag, framebuffer, > framebuffer_red_mask_size); > + mbi_out->fb.green_pos = get_mb2_data(tag, framebuffer, > framebuffer_green_field_position); > + mbi_out->fb.green_size = get_mb2_data(tag, framebuffer, > framebuffer_green_mask_size); > + mbi_out->fb.blue_pos = get_mb2_data(tag, framebuffer, > framebuffer_blue_field_position); > + mbi_out->fb.blue_size = get_mb2_data(tag, framebuffer, > framebuffer_blue_mask_size); > + break; Some of these lines are overly long. Much like you don't have frambuffer_ prefixes on multiboot_info_t field names, you could omit them on multiboot2_tag_framebuffer_t, which would reduce line length some already. You're likely still not going to get around wrapping some of the lines. > --- a/xen/include/xen/multiboot.h > +++ b/xen/include/xen/multiboot.h > @@ -42,6 +42,7 @@ > #define MBI_BIOSCONFIG (_AC(1,u) << 8) > #define MBI_LOADERNAME (_AC(1,u) << 9) > #define MBI_APM (_AC(1,u) << 10) > +#define MBI_FB (_AC(1,u) << 11) >From what I can see in grub's multiboot.h bit 11 is VBE_INFO, while bit 12 is FRAMEBUFFER_INFO. > @@ -101,6 +102,22 @@ typedef struct { > > /* Valid if flags sets MBI_APM */ > u32 apm_table; > + > + /* Valid if flags sets MBI_FB */ > + struct { > + u64 addr; > + u32 pitch; > + u32 width; > + u32 height; > + u8 bpp; > + u8 type; > + u8 red_pos; > + u8 red_size; > + u8 green_pos; > + u8 green_size; > + u8 blue_pos; > + u8 blue_size; > + } fb; > } multiboot_info_t; What you add is not MB1-compatible (VBE fields come first). Furthermore the addition means mbi_reloc() will suddenly copy more data, which I don't think can be assumed to be fully compatible. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |